diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index c380b91a5..fcf2ef4ac 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -127,7 +127,9 @@ jobs: MOCK_ACP_AGENT_BIN: ${{ github.workspace }}/nori-rs/target/mock-acp-out/mock_acp_agent run: | cargo test --profile ci-test --target ${{ matrix.target }} --package tui-pty-e2e - cargo test --profile ci-test --target ${{ matrix.target }} --package nori-acp + cargo test --profile ci-test --target ${{ matrix.target }} --package nori-harness + cargo test --profile ci-test --target ${{ matrix.target }} --package nori-acp-host + cargo test --profile ci-test --target ${{ matrix.target }} --package nori-config cargo test --profile ci-test --target ${{ matrix.target }} --package nori-tui cargo test --profile ci-test --target ${{ matrix.target }} --package nori-cli cargo test --profile ci-test --target ${{ matrix.target }} --package codex-protocol diff --git a/docs.md b/docs.md index e0958f96c..6969b2d18 100644 --- a/docs.md +++ b/docs.md @@ -31,7 +31,7 @@ The project was originally forked from OpenAI Codex CLI and has been adapted to │ nori-tui │ │ Interactive Terminal Interface │ ├────────────────────────┬────────────────────────┤ -│ nori-acp (acp/) │ codex-core (core/) │ +│ nori-harness (harness/)│ codex-core (core/) │ │ ACP Agent Connection │ Config, Auth, Tools │ │ Subprocess Spawning │ Sandbox, Utilities │ ├────────────────────────┴────────────────────────┤ @@ -95,9 +95,8 @@ Nori acts as an MCP client: ### Things to Know -- Nori-authored crates use a `nori-` prefix (`nori-cli`, `nori-tui`, `nori-acp`, `nori-protocol`, `nori-installed`); inherited crates keep the legacy `codex-` prefix from the OpenAI Codex fork and are progressively adopted or removed per `docs/specs/crate-layering.md` -- The `nori-config` feature flag enables Nori-specific configuration paths (`~/.nori/cli/`) instead of the legacy Codex paths (`~/.codex/`) -- The `unstable` feature flag gates experimental ACP features like model switching +- Nori-authored crates use a `nori-` prefix (`nori-cli`, `nori-tui`, `nori-acp`, `nori-acp-host`, `nori-config`, `nori-protocol`, `nori-installed`); inherited crates keep the legacy `codex-` prefix from the OpenAI Codex fork and are progressively adopted or removed per `docs/specs/crate-layering.md` +- Configuration always uses the Nori paths (`~/.nori/cli/`); the old `nori-config` and `unstable` cargo feature flags were removed during the crate-layering cleanup (no cargo feature may change which crate owns a responsibility) - Cross-platform sandboxing is implemented using Landlock (Linux), Seatbelt (macOS), and restricted tokens (Windows) - Snapshot testing with `insta` is used extensively for TUI regression testing - The project has two justfiles: a root `@/justfile` implementing the Shared Local Runner Layer spec (standardized `help`, `dev`, `test`, `doctor` targets) and `@/nori-rs/justfile` for Rust-specific workflows. The root justfile wraps `nori-rs` by running `cd nori-rs && cargo ...` for each target. Both coexist -- run `just` from the repo root for the standard targets, or `cd nori-rs && just` for the Rust-native recipes diff --git a/docs/reference/transcript-format.md b/docs/reference/transcript-format.md new file mode 100644 index 000000000..cbc1e86d1 --- /dev/null +++ b/docs/reference/transcript-format.md @@ -0,0 +1,189 @@ +# Nori transcript format + +Reference for Nori's on-disk session transcript format, for tools that want to +read or write Nori transcripts without depending on the `nori-harness` crate. +Transcripts capture the client-side view of a conversation: what the user +typed, what the assistant responded, and which tools ran. + +Canonical implementation: `nori-rs/harness/src/transcript/` (crate +`nori-harness`) — `types.rs` (schema), `recorder.rs` (writer), `loader.rs` +(reader), `project.rs` (project id derivation). + +## File locations + +Transcripts live under the Nori home directory: + +- `$NORI_HOME` if set, otherwise `~/.nori/cli` (see `nori-config`). + +Layout: + +```text +$NORI_HOME/transcripts/by-project/{project-id}/ + ├── project.json # Project metadata + └── sessions/ + └── {session-id}.jsonl # One file per session +``` + +- `{session-id}` is a UUIDv4 generated at session start. +- Session files are created with mode `0600` on Unix. +- The file is created fresh (truncated) when the session starts, then + appended to line by line. Each line is flushed after writing, but the file + is never fsynced, so the tail of a crashed session may be missing. + +### Project ids and `project.json` + +The project id is 16 lowercase hex characters derived by hashing, in priority +order: + +1. the normalized git remote URL of `origin` (e.g. + `git@github.com:user/repo.git` and `https://github.com/user/repo.git` both + normalize to `github.com/user/repo`), else +2. the git root absolute path, else +3. the canonicalized working directory path. + +The hash is Rust's `DefaultHasher`, which is **not guaranteed stable across +Rust versions**. Treat project ids as opaque directory names: discover +projects by listing `by-project/` and reading each `project.json`, and do not +try to recompute ids yourself. + +`project.json` (pretty-printed JSON, rewritten on every new session in the +project): + +```json +{ + "id": "a1b2c3d4e5f60718", + "name": "my-repo", + "git_remote": "git@github.com:user/my-repo.git", + "git_root": "/home/user/src/my-repo", + "cwd": "/home/user/src/my-repo", + "created_at": "2026-07-03T12:30:45.123Z", + "updated_at": "2026-07-03T12:30:45.123Z" +} +``` + +`git_remote` and `git_root` are `null` outside a git repo. Because the file is +rewritten each session, `created_at` reflects the most recent session, not the +first. + +## Line format + +Session files are JSONL: one self-contained JSON object per line, `\n` +terminated. Every line shares an envelope, with the entry's fields flattened +into the same object: + +- `ts` — ISO 8601 timestamp with millisecond precision, UTC (`Z` suffix), + e.g. `"2026-07-03T12:30:45.123Z"`. +- `v` — schema version, currently `2`. +- `type` — the entry kind (snake_case), plus the entry's own fields. + +The **first line must be a `session_meta` entry**; readers reject files whose +first line is not valid session metadata. + +### `session_meta` (first line) + +```json +{"ts":"2026-07-03T12:30:45.123Z","v":2,"type":"session_meta","session_id":"7f9c2f6a-1c1e-4a9b-9a3e-2f0d8b7c6d5e","project_id":"a1b2c3d4e5f60718","started_at":"2026-07-03T12:30:45.123Z","cwd":"/home/user/src/my-repo","agent":"claude-code","cli_version":"0.9.0","git":{"branch":"main","commit_hash":"1975265abc..."},"acp_session_id":"acp-sess-abc123"} +``` + +Optional fields, omitted when absent: `agent` (ACP agent slug, e.g. +`claude-code`, `codex`, `gemini`), `git` (and within it `branch`, +`commit_hash`), and `acp_session_id` (the ACP agent's own session id, used to +resume via `session/load`). + +### `user` + +```json +{"ts":"2026-07-03T12:31:02.001Z","v":2,"type":"user","id":"msg-001","content":"What files are in src?","attachments":[{"type":"file_path","path":"/tmp/screenshot.png"}]} +``` + +`attachments` is omitted when empty. Each attachment is tagged by `type`: +`file_path` (`path`) or `base64` (`data`, `mime_type`). + +### `assistant` + +```json +{"ts":"2026-07-03T12:31:10.500Z","v":2,"type":"assistant","id":"msg-002","content":[{"type":"thinking","thinking":"Let me check src."},{"type":"text","text":"src contains main.rs and lib.rs."}],"agent":"claude-code"} +``` + +`content` is a list of blocks tagged by `type`: `text` (`text`) or `thinking` +(`thinking`). `agent` is optional. + +### `tool_call` / `tool_result` + +```json +{"ts":"2026-07-03T12:31:05.000Z","v":2,"type":"tool_call","call_id":"call-001","name":"shell","input":{"command":"ls src/"}} +{"ts":"2026-07-03T12:31:05.250Z","v":2,"type":"tool_result","call_id":"call-001","output":"main.rs\nlib.rs","exit_code":0} +``` + +`input` is arbitrary JSON. On results, `truncated` (bool) is omitted when +false and `exit_code` is omitted when not applicable. `call_id` correlates +call and result. + +### `patch_apply` + +```json +{"ts":"2026-07-03T12:31:20.000Z","v":2,"type":"patch_apply","call_id":"call-002","operation":"edit","path":"/home/user/src/my-repo/src/main.rs","success":true} +``` + +`operation` is `edit`, `write`, or `delete`. `error` (string) is present only +on failure. + +### `client_event` + +Wraps a normalized ACP-native event from the `nori-protocol` crate. The +payload lives under `event` and is internally tagged by `event_type` +(snake_case), with the variant's fields flattened alongside the tag: + +```json +{"ts":"2026-07-03T12:31:06.000Z","v":2,"type":"client_event","event":{"event_type":"tool_snapshot","call_id":"call-001","title":"Edit src/main.rs","kind":"edit","phase":"completed","locations":[],"invocation":null,"artifacts":[],"raw_input":null,"raw_output":null}} +``` + +Variants (see `nori-rs/nori-protocol/src/lib.rs` for payload shapes): +`tool_snapshot`, `approval_request`, `message_delta`, `plan_snapshot`, +`session_phase_changed`, `prompt_completed`, `load_completed`, +`queue_changed`, `context_compacted`, `replay_entry`, +`agent_commands_update`, `session_capabilities_changed`, +`session_update_info`, `session_config_update`, `session_mode_changed`, +`thread_goal_updated`, `thread_goal_cleared`, `warning`. This set tracks the +protocol crate and changes more often than the core entry types; readers +should be prepared to skip unknown `event_type`s. + +## Token usage + +Nori transcript entries do not carry token counts. The only usage data that +can appear is the optional `usage` field inside `session_update_info` client +events. The token statistics shown in the TUI footer are parsed from the +underlying agent's *own* transcript files (`~/.claude/projects/`, +`~/.codex/sessions/`, `~/.gemini/tmp/`) by +`nori-rs/harness/src/transcript_discovery.rs`; those are third-party formats +and out of scope here. + +## Versioning and compatibility + +- `v` is currently `2`. The loader does not branch on it; it exists for + forward compatibility. +- **Unknown fields are ignored** (no `deny_unknown_fields` anywhere in the + schema). +- **Unknown or unparseable lines are silently skipped** by the full-transcript + loader — except the first line, which must parse as `session_meta` or the + load fails. +- Blank lines are skipped. +- Lightweight scanners (session pickers, first-user-message preview, user-turn + counts) inspect only the `type` field per line, so extra entry kinds never + break listing. + +Writer expectations for third-party producers: + +- One JSON object per line, no pretty-printing, `\n` terminated. +- First line is `session_meta` with at minimum `session_id`, `project_id`, + `started_at`, `cwd`, `cli_version` (plus the `ts`/`v`/`type` envelope). +- Name the file `{session_id}.jsonl` and keep `session_id` consistent between + the filename and the metadata — loaders derive paths from the id. + +## Reading transcripts programmatically + +If you can take a Rust dependency, use the `nori-harness` crate +(`nori-rs/harness`): `TranscriptLine` / `TranscriptEntry` are the canonical +serde types, `TranscriptLoader` handles discovery, listing, and +tolerant parsing, and `TranscriptRecorder` is the canonical writer. Anything +this document says is a simplification of those types. diff --git a/nori-rs/Cargo.lock b/nori-rs/Cargo.lock index 206ddbbb5..eecfb5b91 100644 --- a/nori-rs/Cargo.lock +++ b/nori-rs/Cargo.lock @@ -220,9 +220,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.100" +version = "1.0.103" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a23eb6b1614318a8071c9b2521f36b424b2c83db5eb3a0fead4a6c0809af6e61" +checksum = "2a4385e2e34eb35d6b3efe798b9eb88096925d87726c0798709bf56d9ed84af3" [[package]] name = "arboard" @@ -925,9 +925,7 @@ dependencies = [ "anyhow", "assert_cmd", "assert_matches", - "async-channel", "base64", - "chardetng", "chrono", "codex-app-server-protocol", "codex-apply-patch", @@ -941,25 +939,21 @@ dependencies = [ "codex-otel", "codex-protocol", "codex-rmcp-client", + "codex-sandbox", "codex-utils-pty", "codex-utils-readiness", "codex-utils-string", - "codex-windows-sandbox", "core-foundation 0.9.4", "core_test_support", "ctor 0.5.0", "diffy", "dirs", "dunce", - "encoding_rs", "env-flags", "escargot", "futures", "http", "keyring", - "landlock", - "libc", - "maplit", "mcp-types", "notify-rust", "once_cell", @@ -971,7 +965,6 @@ dependencies = [ "regex", "regex-lite", "reqwest", - "seccompiler", "serde", "serde_json", "serial_test", @@ -984,7 +977,6 @@ dependencies = [ "time", "tokio", "tokio-test", - "tokio-util", "toml", "toml_edit", "tracing", @@ -1057,6 +1049,8 @@ version = "0.0.0" dependencies = [ "clap", "codex-core", + "codex-protocol", + "codex-sandbox", "landlock", "libc", "seccompiler", @@ -1142,6 +1136,7 @@ dependencies = [ "tracing", "ts-rs", "uuid", + "wildmatch", ] [[package]] @@ -1174,6 +1169,29 @@ dependencies = [ "which", ] +[[package]] +name = "codex-sandbox" +version = "0.0.0" +dependencies = [ + "async-channel", + "chardetng", + "codex-protocol", + "codex-windows-sandbox", + "encoding_rs", + "landlock", + "libc", + "maplit", + "pretty_assertions", + "seccompiler", + "serde", + "serde_json", + "tempfile", + "thiserror 2.0.17", + "tokio", + "tokio-util", + "tracing", +] + [[package]] name = "codex-stdio-to-uds" version = "0.0.0" @@ -1383,6 +1401,8 @@ dependencies = [ "anyhow", "assert_cmd", "codex-core", + "codex-sandbox", + "nori-harness", "notify", "regex-lite", "shlex", @@ -3589,43 +3609,31 @@ dependencies = [ ] [[package]] -name = "nori-acp" +name = "nori-acp-host" version = "0.0.0" dependencies = [ "agent-client-protocol", "agent-client-protocol-schema", "anyhow", - "axum", "base64", - "chrono", - "codex-core", - "codex-git", "codex-protocol", "codex-rmcp-client", "codex-utils-string", "diffy", - "dirs", - "filetime", "futures", "libc", - "nori-protocol", + "nori-config", "oauth2", "pretty_assertions", + "regex", "rmcp", "serde", "serde_json", "serial_test", "tempfile", - "thiserror 2.0.17", "tokio", - "tokio-test", "tokio-util", - "toml", - "toml_edit", "tracing", - "tracing-appender", - "tracing-subscriber", - "uuid", "which", ] @@ -3646,11 +3654,13 @@ dependencies = [ "codex-login", "codex-process-hardening", "codex-protocol", + "codex-sandbox", "codex-stdio-to-uds", "codex-windows-sandbox", "ctor 0.5.0", "libc", - "nori-acp", + "nori-config", + "nori-harness", "nori-tui", "owo-colors", "predicates", @@ -3665,6 +3675,67 @@ dependencies = [ "which", ] +[[package]] +name = "nori-config" +version = "0.0.0" +dependencies = [ + "anyhow", + "codex-protocol", + "dirs", + "pretty_assertions", + "serde", + "serial_test", + "tempfile", + "toml", +] + +[[package]] +name = "nori-harness" +version = "0.0.0" +dependencies = [ + "agent-client-protocol", + "agent-client-protocol-schema", + "anyhow", + "axum", + "base64", + "chrono", + "codex-git", + "codex-protocol", + "codex-rmcp-client", + "codex-utils-string", + "diffy", + "dirs", + "filetime", + "futures", + "libc", + "nori-acp-host", + "nori-config", + "nori-protocol", + "notify-rust", + "oauth2", + "pretty_assertions", + "regex", + "rmcp", + "serde", + "serde_json", + "serial_test", + "shlex", + "tempfile", + "thiserror 2.0.17", + "tokio", + "tokio-test", + "tokio-util", + "toml", + "toml_edit", + "tracing", + "tracing-appender", + "tracing-subscriber", + "tree-sitter", + "tree-sitter-bash", + "uuid", + "which", +] + [[package]] name = "nori-installed" version = "0.0.0" @@ -3672,7 +3743,7 @@ dependencies = [ "anyhow", "chrono", "libc", - "nori-acp", + "nori-harness", "pretty_assertions", "reqwest", "semver", @@ -3717,6 +3788,7 @@ dependencies = [ "codex-login", "codex-protocol", "codex-rmcp-client", + "codex-sandbox", "codex-utils-pty", "codex-windows-sandbox", "color-eyre", @@ -3731,7 +3803,8 @@ dependencies = [ "lazy_static", "libc", "mcp-types", - "nori-acp", + "nori-config", + "nori-harness", "nori-installed", "nori-protocol", "opentelemetry-appender-tracing", diff --git a/nori-rs/Cargo.toml b/nori-rs/Cargo.toml index 5d629443f..6a13aab44 100644 --- a/nori-rs/Cargo.toml +++ b/nori-rs/Cargo.toml @@ -15,7 +15,10 @@ members = [ "login", "mcp-types", "process-hardening", + "acp-host", + "nori-config", "protocol", + "sandbox", "rmcp-client", "stdio-to-uds", "otel", @@ -27,7 +30,7 @@ members = [ "utils/pty", "utils/readiness", "utils/string", - "acp", + "harness", "installed", "mock-acp-agent", "nori-protocol", @@ -43,6 +46,7 @@ version = "0.0.0" # edition. edition = "2024" license = "Apache-2.0" +repository = "https://github.com/tilework-tech/nori-cli" [workspace.dependencies] # Internal @@ -58,6 +62,9 @@ codex-file-search = { path = "file-search" } codex-git = { path = "utils/git" } codex-keyring-store = { path = "keyring-store" } codex-linux-sandbox = { path = "linux-sandbox" } +codex-sandbox = { path = "sandbox" } +nori-acp-host = { path = "acp-host" } +nori-config = { path = "nori-config" } codex-login = { path = "login" } codex-otel = { path = "otel" } codex-process-hardening = { path = "process-hardening" } @@ -75,7 +82,7 @@ codex-utils-string = { path = "utils/string" } codex-windows-sandbox = { path = "windows-sandbox-rs" } core_test_support = { path = "core/tests/common" } mcp-types = { path = "mcp-types" } -nori-acp = { path = "acp" } +nori-harness = { path = "harness" } nori-installed = { path = "installed" } # External diff --git a/nori-rs/README.md b/nori-rs/README.md index 162b0eb26..3780eb405 100644 --- a/nori-rs/README.md +++ b/nori-rs/README.md @@ -16,8 +16,11 @@ progressively adopted or removed (see `docs/specs/crate-layering.md`). |-------|---------| | `cli/` (`nori-cli`) | The shipped `nori` binary: dispatch and subcommands | | `tui/` (`nori-tui`) | Ratatui interactive terminal interface | -| `acp/` (`nori-acp`) | ACP backend: agent registry, connection, session runtime | +| `harness/` (`nori-harness`) | Headless ACP session harness: session runtime, transcripts, hooks | +| `acp-host/` (`nori-acp-host`) | Agent-agnostic ACP hosting: subprocess spawn, wire client, registry | +| `nori-config/` | Nori config layer (`~/.nori/cli/config.toml`) | | `nori-protocol/` | Session-runtime types over the ACP schema | +| `sandbox/` (`codex-sandbox`) | Sandboxed exec engine: Seatbelt, Landlock/seccomp, Windows restricted tokens | | `installed/` (`nori-installed`) | Install detection and analytics | | `mock-acp-agent/` | Mock ACP agent used by tests | | `tui-pty-e2e/` | End-to-end PTY tests driving the real binary | diff --git a/nori-rs/acp-host/Cargo.toml b/nori-rs/acp-host/Cargo.toml new file mode 100644 index 000000000..35aad25f2 --- /dev/null +++ b/nori-rs/acp-host/Cargo.toml @@ -0,0 +1,54 @@ +[package] +edition = "2024" +name = "nori-acp-host" +version = { workspace = true } +license = { workspace = true } +repository = { workspace = true } +description = "Client-side hosting for Agent Client Protocol (ACP) agents: subprocess spawning, the wire connection, agent registry, and event translation" +keywords = ["acp", "agent", "ai", "cli"] +categories = ["development-tools"] + +[lib] +doctest = false +name = "nori_acp_host" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +agent-client-protocol = { workspace = true } +agent-client-protocol-schema = { workspace = true, features = ["unstable"] } +anyhow = { workspace = true } +codex-protocol = { workspace = true } +base64 = { workspace = true } +codex-rmcp-client = { workspace = true } +codex-utils-string = { workspace = true } +diffy = { workspace = true } +futures = { workspace = true } +libc = { workspace = true } +nori-config = { workspace = true } +oauth2 = "5" +regex = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +tokio = { workspace = true, features = [ + "io-util", + "macros", + "process", + "rt-multi-thread", + "sync", + "time", +] } +tokio-util = { workspace = true, features = ["compat"] } +tracing = { workspace = true, features = ["log"] } +which = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +rmcp = { workspace = true, features = [ + "client", + "transport-streamable-http-client-reqwest", +] } +serial_test = { workspace = true } +tempfile = { workspace = true } diff --git a/nori-rs/acp-host/docs.md b/nori-rs/acp-host/docs.md new file mode 100644 index 000000000..51c3b05de --- /dev/null +++ b/nori-rs/acp-host/docs.md @@ -0,0 +1,43 @@ +# Noridoc: nori-acp-host + +Path: @/nori-rs/acp-host + +### Overview + +- Agent-agnostic, client-side ACP hosting machinery, split out of the session harness (now `nori-harness`, then named `nori-acp`) during the crate-layering cleanup (`@/docs/specs/crate-layering.md`). It owns spawning an ACP agent subprocess and speaking JSON-RPC over its stdio (`connection/`, see `@/nori-rs/acp-host/src/connection/docs.md`), the agent registry and distribution resolution (`registry.rs`), the ACP-wire to internal-event bridge (`translator.rs`), file-change/diff helpers (`patch.rs`), and ACP error categorization (`error_category.rs`). +- This is a Layer-0 leaf of the crate layering: it must stay independent of the session harness (`nori-harness`) and any terminal UI so it remains usable and testable by other ACP-ecosystem projects. +- Deliberately harness-free: no session runtime, transcripts, hooks, or goal state. New agent-facing wire behavior belongs here; anything that needs backend session state belongs in `@/nori-rs/harness/`. + +### How it fits into the larger codebase + +``` +nori-tui + | + v +nori-harness (session harness, re-exports this crate) + | + v +nori-acp-host <---> ACP Agent subprocess (JSON-RPC over stdio) +``` + +- `nori-harness` (`@/nori-rs/harness/`) is the primary consumer and re-exports every module (`pub use nori_acp_host::connection;` and friends in `@/nori-rs/harness/src/lib.rs`), so downstream consumers such as `@/nori-rs/tui/` import through `nori_harness` paths. +- Wire/schema types come from the official `agent-client-protocol` SDK; the schema's own `unstable` feature is enabled unconditionally for the Model-category config option. +- Depends on `codex-protocol` (`@/nori-rs/protocol/`) for the internal event vocabulary, `nori-config` (`@/nori-rs/nori-config/`) for agent/MCP/wire-proxy configuration types, and `codex-rmcp-client` (`@/nori-rs/rmcp-client/`) for OAuth token loading in `connection/mcp.rs`. +- Error classification lives here (`AcpErrorCategory`, `categorize_acp_error`); the harness-side user-facing message composition (`enhanced_error_message`) stays in `@/nori-rs/harness/src/backend/`. + +### Core Implementation + +- `connection/` — `AcpConnection::spawn()` launches the agent as a child subprocess, owns its full lifecycle (exit watching, stderr tail, graceful stdin-EOF shutdown), forwards configured MCP servers (`mcp.rs`), optionally wraps the transport in an append-only wire logger (`wire_log.rs`), and delivers everything through one ordered `ConnectionEvent` inbox. See `@/nori-rs/acp-host/src/connection/docs.md`. +- `registry.rs` — data-driven agent registry merging built-in agents (Claude Code, Codex, Gemini) with custom `[[agents]]` config entries; resolves an agent slug to a spawnable `AcpAgentConfig` across npx/bunx/pipx/uvx/local distributions. The registry is process-global state (`AGENT_REGISTRY`, a `RwLock`) initialized once via `initialize_registry()` at startup, with a built-in-defaults fallback when uninitialized. +- `translator.rs` — converts user input into ACP `ContentBlock`s (text plus base64 image blocks) and provides local parsing/display helpers. +- `patch.rs` — diff/patch construction (`create_patch_with_context`) used to normalize file mutations for rendering and transcripts. +- `error_category.rs` — priority-chained substring matching (Auth > Quota > ExecutableNotFound > Initialization > PromptTooLong > ApiServerError > Unknown) over the Debug-formatted error chain; `is_retryable()` marks only server errors and quota limits as transient. + +### Things to Know + +- Detailed behavior documentation for the registry, error categorization, and their harness coupling still lives in `@/nori-rs/harness/docs.md`, which documents the full `nori-harness` public API (this crate's modules are part of that API via re-export). +- The connection layer's child-lifecycle invariants (ordered inbox, stdin-EOF-then-grace shutdown, exit-watcher ownership of the `Child`) are load-bearing for `nori cloud` session release; see `@/nori-rs/acp-host/src/connection/docs.md` before changing teardown behavior. +- `to_acp_mcp_servers()` in `connection/mcp.rs` is not a pure transformation: it eagerly resolves environment variables and loads stored OAuth tokens from the keyring/filesystem at conversion time. +- The dependency direction is `nori-harness -> nori-acp-host`, never the reverse. If a change here needs harness state (session runtime, transcript, goals), thread it in as a parameter or move the logic up to `@/nori-rs/harness/`. + +Created and maintained by Nori. diff --git a/nori-rs/acp/src/connection/acp_connection.rs b/nori-rs/acp-host/src/connection/acp_connection.rs similarity index 99% rename from nori-rs/acp/src/connection/acp_connection.rs rename to nori-rs/acp-host/src/connection/acp_connection.rs index f440d4edc..814a9f217 100644 --- a/nori-rs/acp/src/connection/acp_connection.rs +++ b/nori-rs/acp-host/src/connection/acp_connection.rs @@ -39,9 +39,9 @@ use super::child_lifecycle::STDERR_TAIL_LINES; use super::child_lifecycle::collect_stderr_tail; use super::wire_log::WireDirection; use super::wire_log::WireLogger; -use crate::config::AcpProxyConfig; use crate::registry::AcpAgentConfig; use crate::translator; +use nori_config::AcpProxyConfig; /// Minimum supported ACP protocol version. const MINIMUM_SUPPORTED_VERSION: ProtocolVersion = ProtocolVersion::V1; diff --git a/nori-rs/acp/src/connection/acp_connection_tests.rs b/nori-rs/acp-host/src/connection/acp_connection_tests.rs similarity index 97% rename from nori-rs/acp/src/connection/acp_connection_tests.rs rename to nori-rs/acp-host/src/connection/acp_connection_tests.rs index 9e99e6967..9012b3905 100644 --- a/nori-rs/acp/src/connection/acp_connection_tests.rs +++ b/nori-rs/acp-host/src/connection/acp_connection_tests.rs @@ -40,7 +40,7 @@ async fn recv_approval_request( async fn drive_logged_prompt( config: &crate::registry::AcpAgentConfig, cwd: &std::path::Path, - proxy: crate::config::AcpProxyConfig, + proxy: nori_config::AcpProxyConfig, ) { let conn = AcpConnection::spawn(config, cwd, proxy) .await @@ -80,7 +80,7 @@ async fn test_spawn_and_create_session() { let conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("Failed to spawn AcpConnection"); @@ -105,7 +105,7 @@ async fn test_proxy_logging_records_one_wire_log_per_child_subprocess() { return; }; let temp_dir = tempdir().expect("temp dir"); - let proxy = crate::config::AcpProxyConfig { + let proxy = nori_config::AcpProxyConfig { enabled: true, log_dir: temp_dir.path().join("acp-wire"), }; @@ -165,7 +165,7 @@ async fn test_prompt_receives_text_updates() { let mut conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -215,7 +215,7 @@ async fn test_event_receiver_forwards_session_updates() { let mut conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -262,7 +262,7 @@ async fn test_tool_call_prompt_delivers_final_text_update() { let mut conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -318,7 +318,7 @@ async fn test_session_config_options_after_session_creation() { let conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -348,7 +348,7 @@ async fn test_set_session_config_option_replaces_connection_state() { let conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -390,7 +390,7 @@ async fn test_approval_receiver_forwards_requests() { let mut conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -454,7 +454,7 @@ async fn test_event_receiver_preserves_update_then_approval_order() { let mut conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -552,7 +552,7 @@ async fn test_codex_home_not_inherited() { let mut conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -606,7 +606,7 @@ async fn test_drop_kills_subprocess() { let conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -659,7 +659,7 @@ async fn test_cancel_during_prompt() { let conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -719,7 +719,7 @@ async fn test_sequential_prompt_after_cancel_receives_response() { let mut conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -837,7 +837,7 @@ async fn test_prompt_after_cancel_absorbs_empty_end_turn_tail() { let mut conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn"); @@ -977,7 +977,7 @@ async fn test_shutdown_closes_stdin_and_waits_for_child_exit() { let conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn script agent"); @@ -1023,7 +1023,7 @@ async fn test_shutdown_kills_child_that_outlives_grace() { let conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn script agent"); @@ -1085,7 +1085,7 @@ async fn test_child_exit_emits_event_with_stderr_tail() { let mut conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("spawn script agent"); @@ -1137,7 +1137,7 @@ async fn test_spawn_failure_surfaces_child_stderr() { AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ), ) .await @@ -1150,8 +1150,8 @@ async fn test_spawn_failure_surfaces_child_stderr() { "spawn error must include the child's stderr, got: {err_text}" ); assert_eq!( - crate::backend::categorize_acp_error(&err_text), - crate::backend::AcpErrorCategory::Authentication, + crate::categorize_acp_error(&err_text), + crate::AcpErrorCategory::Authentication, "the surfaced stderr must drive categorization (auth, not init failure)" ); } @@ -1176,7 +1176,7 @@ async fn test_list_sessions_maps_agent_session_info() { let conn = AcpConnection::spawn( &config, temp_dir.path(), - crate::config::AcpProxyConfig::disabled(), + nori_config::AcpProxyConfig::disabled(), ) .await .expect("Failed to spawn AcpConnection"); diff --git a/nori-rs/acp/src/connection/child_lifecycle.rs b/nori-rs/acp-host/src/connection/child_lifecycle.rs similarity index 100% rename from nori-rs/acp/src/connection/child_lifecycle.rs rename to nori-rs/acp-host/src/connection/child_lifecycle.rs diff --git a/nori-rs/acp/src/connection/docs.md b/nori-rs/acp-host/src/connection/docs.md similarity index 81% rename from nori-rs/acp/src/connection/docs.md rename to nori-rs/acp-host/src/connection/docs.md index 5c3413941..2190dca1a 100644 --- a/nori-rs/acp/src/connection/docs.md +++ b/nori-rs/acp-host/src/connection/docs.md @@ -1,6 +1,6 @@ # Noridoc: connection -Path: @/nori-rs/acp/src/connection +Path: @/nori-rs/acp-host/src/connection ### Overview @@ -23,10 +23,11 @@ agent_client_protocol::Lines over child stdin/stdout Local ACP Agent (subprocess, own process group) ``` -- `AcpBackend` in `@/nori-rs/acp/src/backend/` is the sole consumer of `AcpConnection` -- both `AcpBackend::spawn()` and `AcpBackend::resume_session()` call `AcpConnection::spawn()` with an `AcpAgentConfig` resolved from the registry in `@/nori-rs/acp/src/registry.rs` +- This module is part of the `nori-acp-host` crate (`@/nori-rs/acp-host/`), the agent-agnostic Layer-0 leaf; `nori-harness` re-exports it as `nori_harness::connection` +- `AcpBackend` in `@/nori-rs/harness/src/backend/` is the sole consumer of `AcpConnection` -- both `AcpBackend::spawn()` and `AcpBackend::resume_session()` call `AcpConnection::spawn()` with an `AcpAgentConfig` resolved from the registry in `@/nori-rs/acp-host/src/registry.rs` - `nori cloud` rides this exact path: `@/nori-rs/cli/src/cloud.rs` pins a registry entry that runs `nori-handroll cloud-acp`, and that child is spawned here like any other local agent. There is no remote/WebSocket transport in this crate; it lives in the nori-sessions repo - MCP server configuration from `config.toml` is converted to ACP schema types via `mcp.rs` and passed at session creation time -- All transport events (session updates, permission requests, synthetic file-operation updates, and child exits) flow into a single ordered `mpsc::Receiver` consumed by the backend's relay loop in `@/nori-rs/acp/src/backend/spawn_and_relay.rs` +- All transport events (session updates, permission requests, synthetic file-operation updates, and child exits) flow into a single ordered `mpsc::Receiver` consumed by the backend's relay loop in `@/nori-rs/harness/src/backend/spawn_and_relay.rs` - The wire logging layer (`wire_log.rs`) optionally wraps the subprocess transport when `[acp_proxy]` is enabled in config ### Core Implementation @@ -37,18 +38,18 @@ Local ACP Agent (subprocess, own process group) - **stderr capture**: A background task logs each stderr line via tracing and keeps a bounded tail of recent lines. The tail is attached to startup failures and `ChildExited` events so the real cause (e.g. an auth hint) is user-visible - **Startup race**: `spawn()` races the initialization handshake against child death. An agent that exits immediately (e.g. unauthenticated `nori-handroll` printing "run: nori-handroll login") fails the spawn fast with its stderr tail in the error text, which lets `categorize_acp_error` in the backend classify the failure (e.g. Authentication) instead of reporting protocol incompatibility - **Graceful shutdown**: `shutdown()` delegates to `shutdown_with_grace()` with a generous default. Aborting the connection task drops the transport and closes the child's stdin -- stdin EOF is the agent's shutdown signal. The connection waits up to the grace period for a voluntary exit before SIGKILLing the process group, then waits for the watcher to reap so no zombie outlives shutdown -- **MCP server forwarding** (`mcp.rs`): `to_acp_mcp_servers()` converts CLI-configured MCP servers to ACP `McpServer` values, resolving environment variables and OAuth tokens eagerly at conversion time. Disabled servers are filtered out. See `@/nori-rs/acp/docs.md` for details on OAuth token injection +- **MCP server forwarding** (`mcp.rs`): `to_acp_mcp_servers()` converts CLI-configured MCP servers to ACP `McpServer` values, resolving environment variables and OAuth tokens eagerly at conversion time. Disabled servers are filtered out. See `@/nori-rs/harness/docs.md` for details on OAuth token injection - **Ordered event inbox**: Session notifications, permission requests, synthetic file-operation updates, and child exits all flow through one `ConnectionEvent` channel. The backend consumes this single inbox to avoid ordering ambiguity between channels ### Things to Know - The connection sends `InitializeRequest` with `ProtocolVersion::LATEST` and enforces `MINIMUM_SUPPORTED_VERSION = ProtocolVersion::V1` during the initialization handshake in `establish_connection()`; wire/schema types come from `agent_client_protocol_schema::v1` (aliased as `acp`) -- The child process is spawned in its own process group (`setpgid(0, 0)`) and `CODEX_HOME` is stripped from the environment to prevent config parser conflicts (see `@/nori-rs/acp/docs.md` for rationale) +- The child process is spawned in its own process group (`setpgid(0, 0)`) and `CODEX_HOME` is stripped from the environment to prevent config parser conflicts (see `@/nori-rs/harness/docs.md` for rationale) - The stdin-EOF-then-grace shutdown contract exists because agents may need network cleanup on exit; `nori-handroll cloud-acp` releases its broker session on stdin EOF, and the previous immediate-SIGKILL shutdown leaked every cloud session - `Drop` on `AcpConnection` is only a backstop for paths that never ran `shutdown()`: it requests a kill via the recorded pid, with a pid-reuse guard that only signals while the child is unreaped - Without the `ChildExited` event, a dead child is a silent EOF that the ACP SDK layer treats as non-terminal -- pending requests would hang forever. The backend relay turns it into a visible error and fails any in-flight prompt - File write handlers restrict writes to the workspace directory or `/tmp` (canonicalized path check). File read handlers are unrestricted. Both emit synthetic `ToolCall` updates for TUI rendering - `AcpConnection::prompt()` absorbs stale cancel-tail responses (empty `end_turn` responses left over from a previous cancellation) by retrying until streamed updates arrive, keeping the reducer contract unchanged -- `list_sessions()` sends the ACP `session/list` request and drains cursor pagination internally (following `next_cursor` until exhausted, bounded by a page cap so a misbehaving agent that keeps returning a cursor cannot loop unbounded), concatenating every page in agent order. Each ACP `SessionInfo` is mapped into `AcpSessionSummary` (defined in `mod.rs`, re-exported from `nori_acp`), an owned boundary type carrying `session_id`, `cwd`, optional `title`, and optional `updated_at`. The boundary type decouples consumers from the raw ACP schema, letting `@/nori-rs/tui` -- which has no ACP-schema dependency -- render the agent-sourced `/resume` picker. It mirrors `load_session()`'s structure; the agent only honors `session/list` when it advertises the capability (projected as `agent.session_list`, see `@/nori-rs/acp/docs.md`) +- `list_sessions()` sends the ACP `session/list` request and drains cursor pagination internally (following `next_cursor` until exhausted, bounded by a page cap so a misbehaving agent that keeps returning a cursor cannot loop unbounded), concatenating every page in agent order. Each ACP `SessionInfo` is mapped into `AcpSessionSummary` (defined in `mod.rs`, re-exported from `nori_harness`), an owned boundary type carrying `session_id`, `cwd`, optional `title`, and optional `updated_at`. The boundary type decouples consumers from the raw ACP schema, letting `@/nori-rs/tui` -- which has no ACP-schema dependency -- render the agent-sourced `/resume` picker. It mirrors `load_session()`'s structure; the agent only honors `session/list` when it advertises the capability (projected as `agent.session_list`, see `@/nori-rs/harness/docs.md`) Created and maintained by Nori. diff --git a/nori-rs/acp/src/connection/mcp.rs b/nori-rs/acp-host/src/connection/mcp.rs similarity index 99% rename from nori-rs/acp/src/connection/mcp.rs rename to nori-rs/acp-host/src/connection/mcp.rs index ba5f1ceaa..8837cdf2d 100644 --- a/nori-rs/acp/src/connection/mcp.rs +++ b/nori-rs/acp-host/src/connection/mcp.rs @@ -1,6 +1,6 @@ //! Conversion from CLI MCP server config to ACP protocol types. //! -//! The CLI stores MCP server configuration in `codex_core::config::types::McpServerConfig`. +//! The CLI stores MCP server configuration in `codex_protocol::config_types::McpServerConfig`. //! The ACP protocol expects `McpServer` in `NewSessionRequest`. //! This module bridges the two so that CLI-configured MCP servers are forwarded //! to ACP agents at session creation time. @@ -8,8 +8,8 @@ use std::collections::HashMap; use agent_client_protocol_schema::v1 as acp; -use codex_core::config::types::McpServerConfig; -use codex_core::config::types::McpServerTransportConfig; +use codex_protocol::config_types::McpServerConfig; +use codex_protocol::config_types::McpServerTransportConfig; use codex_rmcp_client::OAuthCredentialsStoreMode; use codex_rmcp_client::load_oauth_tokens; use oauth2::TokenResponse; diff --git a/nori-rs/acp/src/connection/mod.rs b/nori-rs/acp-host/src/connection/mod.rs similarity index 98% rename from nori-rs/acp/src/connection/mod.rs rename to nori-rs/acp-host/src/connection/mod.rs index 708b2da63..8d7b30319 100644 --- a/nori-rs/acp/src/connection/mod.rs +++ b/nori-rs/acp-host/src/connection/mod.rs @@ -31,7 +31,7 @@ pub enum ConnectionEvent { }, } -pub(crate) fn session_update_kind(update: &acp::SessionUpdate) -> &'static str { +pub fn session_update_kind(update: &acp::SessionUpdate) -> &'static str { match update { acp::SessionUpdate::AgentMessageChunk(_) => "agent_message_chunk", acp::SessionUpdate::AgentThoughtChunk(_) => "agent_thought_chunk", diff --git a/nori-rs/acp/src/connection/wire_log.rs b/nori-rs/acp-host/src/connection/wire_log.rs similarity index 99% rename from nori-rs/acp/src/connection/wire_log.rs rename to nori-rs/acp-host/src/connection/wire_log.rs index 6d93036bc..d6834100c 100644 --- a/nori-rs/acp/src/connection/wire_log.rs +++ b/nori-rs/acp-host/src/connection/wire_log.rs @@ -13,8 +13,8 @@ use serde_json::Value; use serde_json::json; use tracing::warn; -use crate::config::AcpProxyConfig; use crate::registry::AcpAgentConfig; +use nori_config::AcpProxyConfig; #[derive(Clone)] pub(super) struct WireLogger { diff --git a/nori-rs/acp-host/src/error_category.rs b/nori-rs/acp-host/src/error_category.rs new file mode 100644 index 000000000..0f6c460e6 --- /dev/null +++ b/nori-rs/acp-host/src/error_category.rs @@ -0,0 +1,89 @@ +/// Categories of ACP spawn errors for providing actionable user messages. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AcpErrorCategory { + /// Authentication required or failed + Authentication, + /// Rate limit or quota exceeded + QuotaExceeded, + /// Command/executable not found + ExecutableNotFound, + /// General initialization failure + Initialization, + /// Prompt exceeds the agent's context window + PromptTooLong, + /// API returned a server error (5xx) + ApiServerError, + /// Unknown error (fallback) + Unknown, +} + +impl AcpErrorCategory { + /// Whether this error is transient and worth retrying (e.g. for an + /// unattended loop that should survive a momentary API blip). Server + /// errors are momentary and rate/quota limits ease over time (seconds for + /// rate limits, longer for usage windows); everything else reflects a + /// persistent problem that a retry cannot fix. + pub fn is_retryable(&self) -> bool { + match self { + AcpErrorCategory::ApiServerError | AcpErrorCategory::QuotaExceeded => true, + AcpErrorCategory::Authentication + | AcpErrorCategory::ExecutableNotFound + | AcpErrorCategory::Initialization + | AcpErrorCategory::PromptTooLong + | AcpErrorCategory::Unknown => false, + } + } +} + +/// Categorize an ACP error based on error string patterns. +/// +/// This function analyzes error messages and categorizes them to enable +/// providing actionable instructions to users. +pub fn categorize_acp_error(error: &str) -> AcpErrorCategory { + let error_lower = error.to_lowercase(); + + if error_lower.contains("auth") + || error_lower.contains("-32000") // JSON-RPC auth error code + || error_lower.contains("api key") + || error_lower.contains("unauthorized") + || error_lower.contains("not logged in") + { + AcpErrorCategory::Authentication + } else if error_lower.contains("quota") + || error_lower.contains("rate limit") + || error_lower.contains("rate_limit") // e.g. Anthropic `rate_limit_error` + || error_lower.contains("too many requests") + || error_lower.contains("429") + || error_lower.contains("out of extra usage") + || error_lower.contains("usage limit") + || error_lower.contains("exceeded your usage") + { + AcpErrorCategory::QuotaExceeded + } else if error_lower.contains("command not found") + || (error_lower.contains("no such file") && error_lower.contains("directory")) + || error_lower.contains("os error 2") // ENOENT on Unix + || error_lower.contains("cannot find the path") + // Windows + { + AcpErrorCategory::ExecutableNotFound + } else if error_lower.contains("initialization") + || error_lower.contains("handshake") + || error_lower.contains("protocol") + { + AcpErrorCategory::Initialization + } else if error_lower.contains("prompt is too long") { + AcpErrorCategory::PromptTooLong + } else if error_lower.contains("500") + || error_lower.contains("502") + || error_lower.contains("503") + || error_lower.contains("504") + || error_lower.contains("529") // Anthropic `overloaded_error` + || error_lower.contains("server error") + || error_lower.contains("api_error") + || error_lower.contains("overloaded") + { + AcpErrorCategory::ApiServerError + } else { + AcpErrorCategory::Unknown + } +} diff --git a/nori-rs/acp-host/src/lib.rs b/nori-rs/acp-host/src/lib.rs new file mode 100644 index 000000000..8a1b95b85 --- /dev/null +++ b/nori-rs/acp-host/src/lib.rs @@ -0,0 +1,15 @@ +//! Host side of the Agent Client Protocol: agent registry, subprocess +//! connection management (JSON-RPC over stdio), permission plumbing, and +//! translation between ACP wire types and the internal event vocabulary. +//! +//! Layer 0 of the crate layering (`docs/specs/crate-layering.md`): this crate +//! must stay independent of the session harness and any terminal UI. + +pub mod connection; +mod error_category; +pub mod patch; +pub mod registry; +pub mod translator; + +pub use error_category::AcpErrorCategory; +pub use error_category::categorize_acp_error; diff --git a/nori-rs/acp-host/src/patch.rs b/nori-rs/acp-host/src/patch.rs new file mode 100644 index 000000000..61d923681 --- /dev/null +++ b/nori-rs/acp-host/src/patch.rs @@ -0,0 +1,86 @@ +pub fn create_patch_with_context( + path: &std::path::Path, + cwd: &std::path::Path, + old_text: &str, + new_text: &str, +) -> String { + let full_path = if path.is_absolute() { + path.to_path_buf() + } else { + cwd.join(path) + }; + + let line_offset = if let Ok(file_content) = std::fs::read_to_string(&full_path) { + file_content + .find(old_text) + .or_else(|| file_content.find(new_text)) + .map(|offset| file_content[..offset].lines().count() + 1) + } else { + None + }; + + let patch = diffy::create_patch(old_text, new_text).to_string(); + if let Some(offset) = line_offset + && offset > 1 + { + return adjust_patch_line_numbers(&patch, offset); + } + patch +} + +fn adjust_patch_line_numbers(patch: &str, line_offset: usize) -> String { + let Ok(re) = regex::Regex::new(r"^@@ -(\d+)(,?\d*) \+(\d+)(,?\d*) @@") else { + return patch.to_string(); + }; + let mut result = String::new(); + for line in patch.lines() { + if let Some(caps) = re.captures(line) { + let old_start: usize = caps[1].parse().unwrap_or(1); + let new_start: usize = caps[3].parse().unwrap_or(1); + let old_rest = &caps[2]; + let new_rest = &caps[4]; + + let adjusted_old_start = old_start + line_offset - 1; + let adjusted_new_start = new_start + line_offset - 1; + + result.push_str(&format!( + "@@ -{adjusted_old_start}{old_rest} +{adjusted_new_start}{new_rest} @@\n", + )); + } else { + result.push_str(line); + result.push('\n'); + } + } + result +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn create_patch_with_context_uses_new_text_when_file_is_already_updated() { + let temp_dir = tempfile::tempdir().expect("temp dir"); + let file_path = temp_dir.path().join("story.txt"); + let file_content = (1..=100) + .map(|line| { + if line == 50 { + "line 50 updated\n".to_string() + } else { + format!("line {line}\n") + } + }) + .collect::(); + std::fs::write(&file_path, file_content).expect("write updated file"); + + let patch = create_patch_with_context( + std::path::Path::new("story.txt"), + temp_dir.path(), + "line 50\n", + "line 50 updated\n", + ); + + let hunk_header = patch.lines().find(|line| line.starts_with("@@ ")); + assert_eq!(hunk_header, Some("@@ -50 +50 @@")); + } +} diff --git a/nori-rs/acp/src/registry.rs b/nori-rs/acp-host/src/registry.rs similarity index 98% rename from nori-rs/acp/src/registry.rs rename to nori-rs/acp-host/src/registry.rs index 3ea0ef2ca..35abba335 100644 --- a/nori-rs/acp/src/registry.rs +++ b/nori-rs/acp-host/src/registry.rs @@ -22,8 +22,8 @@ use std::sync::OnceLock; use std::sync::RwLock; use std::time::Duration; -use crate::config::AgentConfigToml; -use crate::config::ResolvedDistribution; +use nori_config::AgentConfigToml; +use nori_config::ResolvedDistribution; /// Default idle timeout for ACP streaming (5 minutes) const DEFAULT_STREAM_IDLE_TIMEOUT: Duration = Duration::from_secs(300); @@ -1228,9 +1228,9 @@ mod tests { #[test] fn test_build_registry_appends_custom_agent() { - use crate::config::AgentConfigToml; - use crate::config::AgentDistributionToml; - use crate::config::PackageDistribution; + use nori_config::AgentConfigToml; + use nori_config::AgentDistributionToml; + use nori_config::PackageDistribution; let custom = AgentConfigToml { name: "Kimi".to_string(), slug: "kimi".to_string(), @@ -1254,9 +1254,9 @@ mod tests { #[test] fn test_build_registry_custom_overrides_builtin() { - use crate::config::AgentConfigToml; - use crate::config::AgentDistributionToml; - use crate::config::LocalDistribution; + use nori_config::AgentConfigToml; + use nori_config::AgentDistributionToml; + use nori_config::LocalDistribution; let custom_claude = AgentConfigToml { name: "My Claude".to_string(), slug: "claude-code".to_string(), @@ -1282,9 +1282,9 @@ mod tests { #[test] fn test_build_registry_rejects_duplicate_custom_slugs() { - use crate::config::AgentConfigToml; - use crate::config::AgentDistributionToml; - use crate::config::PackageDistribution; + use nori_config::AgentConfigToml; + use nori_config::AgentDistributionToml; + use nori_config::PackageDistribution; let agents = vec![ AgentConfigToml { name: "Agent A".to_string(), @@ -1323,9 +1323,9 @@ mod tests { #[serial] fn test_get_agent_config_resolves_custom_uvx_agent() { reset_registry(); - use crate::config::AgentConfigToml; - use crate::config::AgentDistributionToml; - use crate::config::PackageDistribution; + use nori_config::AgentConfigToml; + use nori_config::AgentDistributionToml; + use nori_config::PackageDistribution; let custom = AgentConfigToml { name: "Kimi".to_string(), slug: "kimi".to_string(), @@ -1352,9 +1352,9 @@ mod tests { #[serial] fn test_get_agent_config_resolves_custom_local_agent() { reset_registry(); - use crate::config::AgentConfigToml; - use crate::config::AgentDistributionToml; - use crate::config::LocalDistribution; + use nori_config::AgentConfigToml; + use nori_config::AgentDistributionToml; + use nori_config::LocalDistribution; let custom = AgentConfigToml { name: "Local Agent".to_string(), slug: "local-test".to_string(), @@ -1383,9 +1383,9 @@ mod tests { #[serial] fn test_list_available_agents_includes_custom() { reset_registry(); - use crate::config::AgentConfigToml; - use crate::config::AgentDistributionToml; - use crate::config::PackageDistribution; + use nori_config::AgentConfigToml; + use nori_config::AgentDistributionToml; + use nori_config::PackageDistribution; let custom = AgentConfigToml { name: "Kimi".to_string(), slug: "kimi".to_string(), @@ -1413,9 +1413,9 @@ mod tests { #[serial] fn test_get_agent_display_name_custom_agent() { reset_registry(); - use crate::config::AgentConfigToml; - use crate::config::AgentDistributionToml; - use crate::config::PackageDistribution; + use nori_config::AgentConfigToml; + use nori_config::AgentDistributionToml; + use nori_config::PackageDistribution; let custom = AgentConfigToml { name: "My Custom Agent".to_string(), slug: "my-custom".to_string(), diff --git a/nori-rs/acp/src/translator.rs b/nori-rs/acp-host/src/translator.rs similarity index 99% rename from nori-rs/acp/src/translator.rs rename to nori-rs/acp-host/src/translator.rs index b013474b2..b291062a6 100644 --- a/nori-rs/acp/src/translator.rs +++ b/nori-rs/acp-host/src/translator.rs @@ -739,8 +739,7 @@ pub fn tool_call_to_file_change( let new_string = input.get("new_string").and_then(|v| v.as_str())?; // Generate unified diff using diffy with file context if available - let unified_diff = - codex_core::util::create_patch_with_context(&path, cwd, old_string, new_string); + let unified_diff = crate::patch::create_patch_with_context(&path, cwd, old_string, new_string); Some(( path, diff --git a/nori-rs/app-server-protocol/docs.md b/nori-rs/app-server-protocol/docs.md index eb6619e76..888443a10 100644 --- a/nori-rs/app-server-protocol/docs.md +++ b/nori-rs/app-server-protocol/docs.md @@ -11,7 +11,7 @@ This crate defines the JSON-RPC protocol for external app server communication. Used by: - `@/nori-rs/core/` - for auth mode definitions - `@/nori-rs/tui/` - for auth mode handling -- `@/nori-rs/acp/` - for auth types +- `@/nori-rs/harness/` - for auth types The crate supports both v1 and v2 protocol versions and exports a JSON-RPC lite implementation. diff --git a/nori-rs/app-server-protocol/src/protocol/v1.rs b/nori-rs/app-server-protocol/src/protocol/v1.rs index 9d270ff5f..cdc231c6a 100644 --- a/nori-rs/app-server-protocol/src/protocol/v1.rs +++ b/nori-rs/app-server-protocol/src/protocol/v1.rs @@ -199,8 +199,8 @@ pub struct GitDiffToRemoteResponse { #[serde(rename_all = "camelCase")] pub struct ApplyPatchApprovalParams { pub conversation_id: ConversationId, - /// Use to correlate this with [codex_core::protocol::PatchApplyBeginEvent] - /// and [codex_core::protocol::PatchApplyEndEvent]. + /// Use to correlate this with [codex_protocol::protocol::PatchApplyBeginEvent] + /// and [codex_protocol::protocol::PatchApplyEndEvent]. pub call_id: String, pub file_changes: HashMap, /// Optional explanatory reason (e.g. request for extra write access). @@ -220,8 +220,8 @@ pub struct ApplyPatchApprovalResponse { #[serde(rename_all = "camelCase")] pub struct ExecCommandApprovalParams { pub conversation_id: ConversationId, - /// Use to correlate this with [codex_core::protocol::ExecCommandBeginEvent] - /// and [codex_core::protocol::ExecCommandEndEvent]. + /// Use to correlate this with [codex_protocol::protocol::ExecCommandBeginEvent] + /// and [codex_protocol::protocol::ExecCommandEndEvent]. pub call_id: String, pub command: Vec, pub cwd: PathBuf, diff --git a/nori-rs/apply-patch/docs.md b/nori-rs/apply-patch/docs.md index 541a0a06c..f62262195 100644 --- a/nori-rs/apply-patch/docs.md +++ b/nori-rs/apply-patch/docs.md @@ -10,7 +10,7 @@ The apply-patch crate implements a custom patch format for AI-driven file modifi This crate is used by: - `@/nori-rs/core/` - for applying file patches requested by AI models -- `@/nori-rs/acp/` - for patch validation and preview generation +- `@/nori-rs/harness/` - for patch validation and preview generation The crate can also be run as a standalone executable for testing. diff --git a/nori-rs/async-utils/docs.md b/nori-rs/async-utils/docs.md index c771c7ba7..4dc7d8954 100644 --- a/nori-rs/async-utils/docs.md +++ b/nori-rs/async-utils/docs.md @@ -8,7 +8,7 @@ The async-utils crate provides async utilities for Tokio. Currently it contains ### How it fits into the larger codebase -Used throughout the workspace where async operations need cancellation support, particularly in `@/nori-rs/core/` and `@/nori-rs/acp/`. +Used throughout the workspace where async operations need cancellation support, particularly in `@/nori-rs/core/` and `@/nori-rs/harness/`. ### Core Implementation diff --git a/nori-rs/cli/Cargo.toml b/nori-rs/cli/Cargo.toml index b4d0d74a7..a2a4dad3b 100644 --- a/nori-rs/cli/Cargo.toml +++ b/nori-rs/cli/Cargo.toml @@ -25,11 +25,13 @@ login = ["dep:codex-login", "nori-tui/login"] anyhow = { workspace = true } clap = { workspace = true, features = ["derive"] } clap_complete = { workspace = true } -nori-acp = { workspace = true } +nori-harness = { workspace = true } +nori-config = { workspace = true } codex-app-server-protocol = { workspace = true } codex-arg0 = { workspace = true } codex-common = { workspace = true, features = ["cli"] } codex-core = { workspace = true } +codex-sandbox = { workspace = true } codex-execpolicy = { workspace = true } codex-login = { workspace = true, optional = true } codex-process-hardening = { workspace = true } diff --git a/nori-rs/cli/docs.md b/nori-rs/cli/docs.md index ceb927c68..78e20f05b 100644 --- a/nori-rs/cli/docs.md +++ b/nori-rs/cli/docs.md @@ -10,9 +10,10 @@ The `nori-cli` crate is the main binary that provides the `nori` command. It ser This crate is the primary entry point that ties together the core crates: -- **Always included:** `nori-tui`, `nori-acp`, `codex-core` +- **Always included:** `nori-tui`, `nori-harness`, `nori-config`, `codex-core`, `codex-sandbox` - **Optional via features:** `codex-login` - **Uses** `codex-arg0` for arg0-based dispatch (Linux sandbox embedding) +- **Uses** `codex-sandbox` (`@/nori-rs/sandbox/`) for the `nori sandbox` debug subcommand's seatbelt/landlock/windows spawn helpers ### Core Implementation @@ -51,7 +52,7 @@ match subcommand { - `cloud_agent_config()` builds a synthetic registry entry (slug `nori-cloud`): a local distribution running ` cloud-acp`, with the read-only `[cloud] broker_url` from `config.toml` (when present) translated to a `NORI_BROKER_URL` environment variable on the child, and an auth hint pointing at `nori-handroll login` - The dispatch in `main.rs` forces `interactive.agent = "nori-cloud"` AFTER flag merging, so `--agent` cannot bypass Sessions, and passes the entry via the clap-skipped `TuiCli.extra_agents` field (see `@/nori-rs/tui/src/cli.rs`) - From there the handroll child rides the ordinary local-agent path end to end: registry lookup, `AcpConnection::spawn()`, and unconditional local transcript recording (duplicating the broker's server-side recording is intentional) -- Auth, broker REST, session acquisition/release, and tunnel transport all live inside `nori-handroll cloud-acp`. Clean release relies on the graceful stdin-EOF shutdown contract in `@/nori-rs/acp/src/connection/acp_connection.rs` +- Auth, broker REST, session acquisition/release, and tunnel transport all live inside `nori-handroll cloud-acp`. Clean release relies on the graceful stdin-EOF shutdown contract in `@/nori-rs/acp-host/src/connection/acp_connection.rs` - TUI flags such as `--agent`, `--profile`, `--sandbox` can still be passed after `cloud` (only `--agent` is overridden) **Debug Sandbox** (`debug_sandbox.rs`): Implementation of the sandbox testing commands. diff --git a/nori-rs/cli/src/cloud.rs b/nori-rs/cli/src/cloud.rs index 323ed3fd0..db16c41ce 100644 --- a/nori-rs/cli/src/cloud.rs +++ b/nori-rs/cli/src/cloud.rs @@ -9,7 +9,7 @@ use std::path::Path; use std::path::PathBuf; -use nori_acp::config::AgentConfigToml; +use nori_config::AgentConfigToml; /// Registry slug for the pinned cloud agent. pub const CLOUD_AGENT_SLUG: &str = "nori-cloud"; @@ -75,8 +75,8 @@ pub fn cloud_agent_config(handroll_bin: &Path, broker_url: Option<&str>) -> Agen AgentConfigToml { name: "Nori Cloud".to_string(), slug: CLOUD_AGENT_SLUG.to_string(), - distribution: nori_acp::config::AgentDistributionToml { - local: Some(nori_acp::config::LocalDistribution { + distribution: nori_config::AgentDistributionToml { + local: Some(nori_config::LocalDistribution { command: handroll_bin.to_string_lossy().into_owned(), args: vec!["cloud-acp".to_string()], env, @@ -177,7 +177,7 @@ mod tests { .resolve() .expect("distribution must be valid"); match resolved { - nori_acp::config::ResolvedDistribution::Local { command, args, env } => { + nori_config::ResolvedDistribution::Local { command, args, env } => { assert_eq!(command, "/opt/bin/nori-handroll"); assert_eq!(args, vec!["cloud-acp".to_string()]); assert!( @@ -209,7 +209,7 @@ mod tests { .resolve() .expect("distribution must be valid"); match resolved { - nori_acp::config::ResolvedDistribution::Local { env, .. } => { + nori_config::ResolvedDistribution::Local { env, .. } => { assert_eq!( env.get("NORI_BROKER_URL").map(String::as_str), Some("http://broker.test:19400"), diff --git a/nori-rs/cli/src/debug_sandbox.rs b/nori-rs/cli/src/debug_sandbox.rs index 26fecd55c..4a4ca4bfe 100644 --- a/nori-rs/cli/src/debug_sandbox.rs +++ b/nori-rs/cli/src/debug_sandbox.rs @@ -8,12 +8,12 @@ use std::path::PathBuf; use codex_common::CliConfigOverrides; use codex_core::config::Config; use codex_core::config::ConfigOverrides; -use codex_core::exec_env::create_env; -use codex_core::landlock::spawn_command_under_linux_sandbox; -#[cfg(target_os = "macos")] -use codex_core::seatbelt::spawn_command_under_seatbelt; -use codex_core::spawn::StdioPolicy; use codex_protocol::config_types::SandboxMode; +use codex_sandbox::exec_env::create_env; +use codex_sandbox::landlock::spawn_command_under_linux_sandbox; +#[cfg(target_os = "macos")] +use codex_sandbox::seatbelt::spawn_command_under_seatbelt; +use codex_sandbox::spawn::StdioPolicy; use crate::LandlockCommand; use crate::SeatbeltCommand; diff --git a/nori-rs/cli/src/main.rs b/nori-rs/cli/src/main.rs index 11559332e..6fe0325d0 100644 --- a/nori-rs/cli/src/main.rs +++ b/nori-rs/cli/src/main.rs @@ -3,8 +3,6 @@ use clap::Parser; use codex_arg0::arg0_dispatch_or_else; use codex_common::CliConfigOverrides; use codex_execpolicy::ExecPolicyCheckCommand; -use nori_acp::find_nori_home; -use nori_acp::init_rolling_file_tracing; use nori_cli::LandlockCommand; use nori_cli::SeatbeltCommand; use nori_cli::WindowsCommand; @@ -20,6 +18,8 @@ use nori_cli::login::run_login_with_chatgpt; use nori_cli::login::run_login_with_device_code; #[cfg(feature = "login")] use nori_cli::login::run_logout; +use nori_config::find_nori_home; +use nori_harness::init_rolling_file_tracing; use nori_tui::AppExitInfo; use nori_tui::Cli as TuiCli; @@ -235,7 +235,7 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec anyhow::Result<()> { } } else { // Fall back to npx/bunx if not in PATH - use nori_acp::registry::detect_preferred_package_manager; + use nori_harness::registry::detect_preferred_package_manager; let package_manager = detect_preferred_package_manager(); let runner = package_manager.command(); // "npx" or "bunx" @@ -524,7 +524,7 @@ async fn cli_main(codex_linux_sandbox_exe: Option) -> anyhow::Result<() std::env::var_os("NORI_HANDROLL_BIN"), std::env::var_os("PATH"), )?; - let nori_config = nori_acp::config::NoriConfig::load().unwrap_or_default(); + let nori_config = nori_config::NoriConfig::load().unwrap_or_default(); merge_interactive_cli_flags(&mut interactive, cloud_cmd.config_overrides); prepend_config_flags( @@ -620,8 +620,8 @@ fn merge_interactive_cli_flags(interactive: &mut TuiCli, subcommand_cli: TuiCli) #[cfg(test)] mod tests { use super::*; - use codex_core::protocol::TokenUsage; use codex_protocol::ConversationId; + use codex_protocol::protocol::TokenUsage; use pretty_assertions::assert_eq; fn finalize_resume_from_args(args: &[&str]) -> TuiCli { diff --git a/nori-rs/cli/tests/nori_acp_crate.rs b/nori-rs/cli/tests/nori_harness_crate.rs similarity index 51% rename from nori-rs/cli/tests/nori_acp_crate.rs rename to nori-rs/cli/tests/nori_harness_crate.rs index 6384f57ad..4e03421f1 100644 --- a/nori-rs/cli/tests/nori_acp_crate.rs +++ b/nori-rs/cli/tests/nori_harness_crate.rs @@ -1,6 +1,6 @@ -use nori_acp::find_nori_home; +use nori_config::find_nori_home; #[test] -fn nori_acp_crate_is_available_to_cli() { +fn nori_harness_crate_is_available_to_cli() { let _ = find_nori_home as fn() -> anyhow::Result; } diff --git a/nori-rs/common/docs.md b/nori-rs/common/docs.md index e98ac8cef..59d379774 100644 --- a/nori-rs/common/docs.md +++ b/nori-rs/common/docs.md @@ -11,7 +11,7 @@ The common crate provides shared utilities used across multiple Nori components. Used by: - `@/nori-rs/tui/` - for CLI argument parsing, model presets, fuzzy matching - `@/nori-rs/core/` - (indirectly via config types) -- `@/nori-rs/acp/` - for model presets +- `@/nori-rs/harness/` - for model presets ### Core Implementation diff --git a/nori-rs/common/src/approval_mode_cli_arg.rs b/nori-rs/common/src/approval_mode_cli_arg.rs index e8c068268..0523c84f3 100644 --- a/nori-rs/common/src/approval_mode_cli_arg.rs +++ b/nori-rs/common/src/approval_mode_cli_arg.rs @@ -3,7 +3,7 @@ use clap::ValueEnum; -use codex_core::protocol::AskForApproval; +use codex_protocol::protocol::AskForApproval; #[derive(Clone, Copy, Debug, ValueEnum)] #[value(rename_all = "kebab-case")] diff --git a/nori-rs/common/src/approval_presets.rs b/nori-rs/common/src/approval_presets.rs index 2e7213bbb..1a0caffaa 100644 --- a/nori-rs/common/src/approval_presets.rs +++ b/nori-rs/common/src/approval_presets.rs @@ -1,5 +1,5 @@ -use codex_core::protocol::AskForApproval; -use codex_core::protocol::SandboxPolicy; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::SandboxPolicy; /// A simple preset pairing an approval policy with a sandbox policy. #[derive(Debug, Clone)] diff --git a/nori-rs/common/src/model_presets.rs b/nori-rs/common/src/model_presets.rs index 1c604b18e..b9383f194 100644 --- a/nori-rs/common/src/model_presets.rs +++ b/nori-rs/common/src/model_presets.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use codex_app_server_protocol::AuthMode; -use codex_core::protocol_config_types::ReasoningEffort; +use codex_protocol::config_types::ReasoningEffort; use once_cell::sync::Lazy; pub const HIDE_GPT5_1_MIGRATION_PROMPT_CONFIG: &str = "hide_gpt5_1_migration_prompt"; diff --git a/nori-rs/common/src/sandbox_mode_cli_arg.rs b/nori-rs/common/src/sandbox_mode_cli_arg.rs index fa5662ce6..de7375f67 100644 --- a/nori-rs/common/src/sandbox_mode_cli_arg.rs +++ b/nori-rs/common/src/sandbox_mode_cli_arg.rs @@ -1,6 +1,6 @@ //! Standard type to use with the `--sandbox` (`-s`) CLI option. //! -//! This mirrors the variants of [`codex_core::protocol::SandboxPolicy`], but +//! This mirrors the variants of [`codex_protocol::protocol::SandboxPolicy`], but //! without any of the associated data so it can be expressed as a simple flag //! on the command-line. Users that need to tweak the advanced options for //! `workspace-write` can continue to do so via `-c` overrides or their diff --git a/nori-rs/common/src/sandbox_summary.rs b/nori-rs/common/src/sandbox_summary.rs index 66e00cd45..cc3c547bf 100644 --- a/nori-rs/common/src/sandbox_summary.rs +++ b/nori-rs/common/src/sandbox_summary.rs @@ -1,4 +1,4 @@ -use codex_core::protocol::SandboxPolicy; +use codex_protocol::protocol::SandboxPolicy; pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { match sandbox_policy { diff --git a/nori-rs/core/Cargo.toml b/nori-rs/core/Cargo.toml index d2c6f2bec..b2d1bcb54 100644 --- a/nori-rs/core/Cargo.toml +++ b/nori-rs/core/Cargo.toml @@ -14,10 +14,8 @@ workspace = true [dependencies] anyhow = { workspace = true } -async-channel = { workspace = true } base64 = { workspace = true } chrono = { workspace = true, features = ["serde"] } -chardetng = { workspace = true } codex-app-server-protocol = { workspace = true } codex-apply-patch = { workspace = true } codex-async-utils = { workspace = true } @@ -27,20 +25,18 @@ codex-git = { workspace = true } codex-keyring-store = { workspace = true } codex-otel = { workspace = true, features = ["otel"] } codex-protocol = { workspace = true } +codex-sandbox = { workspace = true } codex-rmcp-client = { workspace = true } codex-utils-pty = { workspace = true } codex-utils-readiness = { workspace = true } codex-utils-string = { workspace = true } -codex-windows-sandbox = { package = "codex-windows-sandbox", path = "../windows-sandbox-rs" } diffy = { workspace = true } dirs = { workspace = true } dunce = { workspace = true } env-flags = { workspace = true } -encoding_rs = { workspace = true } futures = { workspace = true } http = { workspace = true } keyring = { workspace = true, features = ["crypto-rust"] } -libc = { workspace = true } mcp-types = { workspace = true } os_info = { workspace = true } rand = { workspace = true } @@ -71,7 +67,6 @@ tokio = { workspace = true, features = [ "rt-multi-thread", "signal", ] } -tokio-util = { workspace = true, features = ["rt"] } toml = { workspace = true } toml_edit = { workspace = true } tracing = { workspace = true, features = ["log"] } @@ -86,8 +81,6 @@ deterministic_process_ids = [] [target.'cfg(target_os = "linux")'.dependencies] -landlock = { workspace = true } -seccompiler = { workspace = true } keyring = { workspace = true, features = ["linux-native-async-persistent"] } [target.'cfg(target_os = "macos")'.dependencies] @@ -124,7 +117,6 @@ codex-core = { path = ".", features = ["deterministic_process_ids"] } core_test_support = { workspace = true } ctor = { workspace = true } escargot = { workspace = true } -maplit = { workspace = true } predicates = { workspace = true } pretty_assertions = { workspace = true } serial_test = { workspace = true } diff --git a/nori-rs/core/docs.md b/nori-rs/core/docs.md index 8feb3a0bc..67b141088 100644 --- a/nori-rs/core/docs.md +++ b/nori-rs/core/docs.md @@ -4,38 +4,39 @@ Path: @/nori-rs/core ### Overview -The core crate provides foundational functionality shared across Nori components: configuration management, authentication, command execution with sandboxing, compaction utilities, and MCP (Model Context Protocol) server connections. This is the largest crate in the workspace and contains most shared business logic. +The core crate is shared infrastructure inherited from the Codex fork, slimmed down by the crate-layering cleanup (`@/docs/specs/crate-layering.md`) to what the `nori` binary actually uses: configuration loading and editing, authentication, MCP auth helpers, and model/provider metadata. It is no longer a business-logic hub -- session semantics live in `@/nori-rs/harness/` (which does not depend on this crate at all), and the sandboxed-execution engine now lives in `@/nori-rs/sandbox/`. ### How it fits into the larger codebase ``` -nori-tui / nori-acp +nori-tui / nori-cli / codex-login | v codex-core - / | \ - v v v -config auth exec/sandboxing + / | \ + v v v +config auth codex-sandbox (errors, platform sandbox selection) | v codex-protocol (types) ``` The core crate is depended on by: -- `@/nori-rs/tui/` - for config loading, auth management, and shared types -- `@/nori-rs/acp/` - for config types and auth helpers +- `@/nori-rs/tui/` - for config loading, auth management, and git info +- `@/nori-rs/cli/` - for config and auth - `@/nori-rs/login/` - for auth primitives +- `@/nori-rs/harness/` does **not** depend on core; the ACP-facing helpers it used to import (user notifications, custom prompts, shell/command parsing, compact constants, patch construction) now live in that crate Key integrations: -- Uses `codex-protocol` for wire types (`@/nori-rs/protocol/`) -- Uses `codex-execpolicy` for execution policy parsing (`@/nori-rs/execpolicy/`) -- Uses `codex-apply-patch` for file patching (`@/nori-rs/apply-patch/`) -- Uses `codex-rmcp-client` for MCP server communication (`@/nori-rs/rmcp-client/`) +- Uses `codex-protocol` for shared types (`@/nori-rs/protocol/`), including the MCP server config types and shell environment policy types defined in its `config_types` module. Core previously re-exported `codex_protocol`'s protocol modules; those re-exports were deleted, so every crate imports `codex_protocol` directly. +- Uses `codex-sandbox` (`@/nori-rs/sandbox/`) for the shared error types (`CodexErr`, `RefreshTokenFailedError` in `auth.rs`), `TruncationPolicy` (in `model_family.rs`), and platform-sandbox selection during config resolution (`get_platform_sandbox` / `set_windows_sandbox_enabled` in `config/mod.rs`). The dependency direction is core -> sandbox, never the reverse. +- Uses `codex-rmcp-client` for MCP OAuth flows (`@/nori-rs/rmcp-client/`) +- Uses `codex-keyring-store` for persistent auth token storage (`@/nori-rs/keyring-store/`) ### Core Implementation **Configuration** (`config/`, `config_loader/`): Loads and merges configuration from: -1. Global config at `~/.codex/config.toml` (or `~/.nori/cli/config.toml` with nori-config feature) +1. Global config at `$CODEX_HOME/config.toml` (the `nori` binary points `CODEX_HOME` at `~/.nori/cli`, so core and the Nori config layer in `@/nori-rs/nori-config/src/` read the same file) 2. Project-local config at `/.codex/config.toml` 3. Command-line overrides @@ -64,56 +65,26 @@ The builder is used by the TUI layer (`@/nori-rs/tui/`) to persist user preferen - ChatGPT login flow with OAuth - Keyring storage for persistent tokens (`codex-keyring-store`) -**Command Execution** (`exec.rs`, `sandboxing/`): Executes shell commands with optional sandboxing: -- Linux: Landlock LSM (`landlock.rs`) + seccomp -- macOS: Seatbelt sandbox profiles (`seatbelt.rs`) -- Windows: Restricted process tokens (`codex-windows-sandbox`) +**Command Execution**: No longer lives here. The exec engine, sandbox wrappers, spawn helpers, and error types moved to the `codex-sandbox` crate -- see `@/nori-rs/sandbox/docs.md`. -**Command Safety** (`command_safety/`): Determines whether shell commands are known-safe and can be auto-approved without user confirmation, based on execution policy rules from `@/nori-rs/execpolicy/`. - -**Custom Prompts** (`custom_prompts.rs`): Discovers and executes user-authored custom prompts from a directory. Two kinds of prompts are supported: - -| Kind | Extensions | Behavior | -|------|-----------|----------| -| Markdown | `.md` | Content is read, frontmatter parsed for `description` and `argument_hint`, body becomes the prompt template | -| Script | `.sh`, `.py`, `.js` | File is discovered with an assigned interpreter; content is empty at discovery time; execution happens later via `execute_script()` | - -`discover_prompts_in()` scans a directory for supported file extensions, assigns a `CustomPromptKind` (from `@/nori-rs/protocol/src/custom_prompts.rs`), and returns sorted `CustomPrompt` structs. Scripts are assigned interpreters: `.sh` -> `bash`, `.py` -> `python3`, `.js` -> `node`. - -`execute_script()` runs a `Script`-kind prompt via its interpreter (e.g. `bash script.sh arg1 arg2`), captures stdout, and enforces a configurable timeout. Returns `Ok(stdout)` on zero exit or `Err(message)` on non-zero exit, I/O error, or timeout. - -**MCP Integration** (`mcp/`, `mcp_connection_manager.rs`): Connects to MCP servers (defined in config) to provide additional tools to the AI model. The `McpServerTransportConfig::StreamableHttp` variant supports two OAuth credential modes: dynamic client registration (the default, handled by `rmcp`'s `OAuthState`) and pre-configured client credentials via optional `client_id` and `client_secret_env_var` fields for servers that do not support dynamic registration (e.g., Slack). The `client_secret_env_var` field follows the same env-var-name pattern as `bearer_token_env_var` -- the actual secret is resolved from the environment at runtime. These fields are rejected during deserialization for stdio transport. +**MCP Auth Helpers** (`mcp/`): Provides OAuth/auth-status helpers for MCP servers defined in config (e.g. `mcp::auth::compute_auth_statuses()`, used by the TUI's MCP server picker). The `McpServerConfig` and `McpServerTransportConfig` types themselves are defined in `codex_protocol::config_types` (`@/nori-rs/protocol/src/config_types.rs`) so that `@/nori-rs/harness/` can consume them without depending on core; core re-exports them through `config/types.rs` for its own config code. The `McpServerTransportConfig::StreamableHttp` variant supports two OAuth credential modes: dynamic client registration (the default, handled by `rmcp`'s `OAuthState`) and pre-configured client credentials via optional `client_id` and `client_secret_env_var` fields for servers that do not support dynamic registration (e.g., Slack). The `client_secret_env_var` field follows the same env-var-name pattern as `bearer_token_env_var` -- the actual secret is resolved from the environment at runtime. These fields are rejected during deserialization for stdio transport. **Data Flow (ACP path):** ``` -User Input -> Op (UserTurn) -> AcpBackend (@/nori-rs/acp) -> Agent (JSON-RPC via subprocess or WebSocket) +User Input -> Op (UserTurn) -> AcpBackend (@/nori-rs/harness) -> Agent (JSON-RPC via subprocess stdio) | v Event (TurnStart/Delta/Complete) <- Response Processing <- Tool Execution ``` -ACP (Agent Context Protocol) integration is handled in `@/nori-rs/acp`, not embedded in core. The core crate provides shared infrastructure (config, auth, tool specs, sandboxing, compaction utilities) that the ACP backend consumes. +ACP (Agent Context Protocol) integration is handled in `@/nori-rs/harness`, not embedded in core. Core provides infrastructure (config, auth) to the frontends; the ACP backend itself does not import core -- it shares only the `codex-protocol` type vocabulary. **Shared Types Module (`tool_types.rs`):** Types and constants needed across modules are collected in `tool_types.rs`. This includes `ApplyPatchToolType`, `ConfigShellToolType`, and `CODEX_APPLY_PATCH_ARG1`. The constant `CODEX_APPLY_PATCH_ARG1` is re-exported from `lib.rs` because `codex-arg0` (`@/nori-rs/arg0/`) imports it for argv dispatch and Windows batch scripts. **Model Provider Info (`model_provider_info.rs`):** A pure configuration type defining `ModelProviderInfo` (provider name, optional env-key reference, and retry/timeout settings). The ACP backend communicates over subprocess stdio rather than HTTP, so HTTP-specific fields (base URL, headers, query params, bearer token) have been removed. Built-in providers (OpenAI, Ollama, LMStudio) are defined in `built_in_model_providers()`. User-defined providers in `config.toml` may still include removed fields; serde silently ignores them for backwards compatibility. -**Compact Utilities (`compact.rs`):** Provides shared compaction constants for conversation summarization: `SUMMARIZATION_PROMPT` and `SUMMARY_PREFIX`, which are loaded from prompt templates in `templates/compact/`. - -**User Notifications:** - -The `user_notification.rs` module provides OS-level notification support: - -| Notification Type | Title | Body Content | -|-------------------|-------|--------------| -| `AgentTurnComplete` | "Nori: Task Complete" | Last assistant message, or "Completed: {input}" fallback | -| `AwaitingApproval` | "Nori: Approval Required" | Truncated command and cwd | -| `Idle` | "Nori: Session Idle" | Idle duration in seconds | - -Notification modes: -1. **Native notifications** (`use_native: true`): Uses `notify-rust` for desktop notifications. All calls to `send_native()` are non-blocking -- they spawn a background thread to call `notif.show()`, because some platforms (notably macOS) block synchronously on that call. On X11 Linux, the spawned thread also handles click-to-focus via `wmctrl` or `xdotool`. The `use_native` flag is controlled by `OsNotifications` in the ACP config layer (`@/nori-rs/acp/src/config/types.rs`). -2. **External script** (`notify_command` configured): Invokes user-specified command with JSON payload. +**TUI Display Settings:** Core's `Config::tui_notifications` is a simple `bool` that controls whether the TUI sends OSC 9 terminal escape sequence notifications. It derives its value from the ACP config's `TerminalNotifications` enum during config loading. Core also carries TUI display booleans such as `animations` and `custom_working_messages`; the latter mirrors `[tui].custom_working_messages` from Nori config so the TUI can choose between rotating custom working headers and the plain `Working` label without re-reading config. Core additionally carries `Config::custom_working_message_list: Vec`, mirroring `[tui].custom_working_message_list`; when non-empty and `custom_working_messages` is `true`, the TUI samples this user list instead of the builtin whimsical messages. @@ -121,17 +92,23 @@ Core's `Config::tui_notifications` is a simple `bool` that controls whether the **Module Structure Convention:** -Large modules use a directory layout (`foo/mod.rs` + submodules) instead of a single `foo.rs` file. This separates concerns and keeps individual files manageable. Modules using this pattern include `parse_command/`, `rollout/`, and `config/` (which also has a `notifications_tests.rs` alongside `tests.rs`). Test submodules use `tests/mod.rs` + `tests/part*.rs` for large test suites (e.g., `config/tests/`). Integration tests like `tests/suite/compact/` and `tests/suite/client/` also use the `mod.rs` + `part*.rs` pattern. +Large modules use a directory layout (`foo/mod.rs` + submodules) instead of a single `foo.rs` file. This separates concerns and keeps individual files manageable. Modules using this pattern include `config/`, `auth/`, and `mcp/`. Test submodules use `tests/mod.rs` + `tests/part*.rs` for large test suites (e.g., `config/tests/`). + +**What moved out during the crate-layering cleanup** (`@/docs/specs/crate-layering.md`): + +- Dead Codex-engine subsystems were deleted outright: rollout recording (superseded by the transcript recorder in `@/nori-rs/harness/src/transcript/`), command-safety auto-approval, turn diff tracking, event mapping, and user-instruction plumbing. +- ACP-facing leaf helpers moved into `@/nori-rs/harness/src/`: user notifications, custom prompt discovery, shell/command parsing (`parse_command`, `shell`, `bash`, `powershell`), the compact summarization constants and templates, and `create_patch_with_context` (formerly in `util.rs`, which now only holds error-message parsing helpers). +- `McpServerConfig`/`McpServerTransportConfig` and the shell environment policy types (`ShellEnvironmentPolicy` and friends) moved down into `codex_protocol::config_types`; core re-exports them for its own config code. +- The sandboxed-execution engine moved into `codex-sandbox` (`@/nori-rs/sandbox/`): `exec`, `exec_env`, `spawn`, `safety`, `sandboxing/`, `seatbelt` (+ `.sbpl` policies), `landlock`, `text_encoding`, `truncate`, and `error` (`CodexErr`/`SandboxErr`). Its integration tests (exec, seatbelt, text encoding) moved out of `core/tests/suite/` at the same time. Frontends that need exec/sandbox functionality import `codex_sandbox` directly. + +Other notes: -- The `deterministic_process_ids` feature is for testing only - produces predictable IDs instead of UUIDs -- Sandbox policies are defined in `.sbpl` files for macOS Seatbelt - Config uses TOML with optional environment variable expansion - Auth tokens are stored in the system keyring with fallback to file storage -- The conversation history is stored in `~/.codex/conversations/` (or `~/.nori/cli/conversations/`) -- Error types are defined in `error.rs` and use `thiserror` +- Core has no `error` module of its own; it uses the `thiserror` types from `codex_sandbox::error` **Test Suite:** -The integration test suite in `@/nori-rs/core/tests/suite` covers auth refresh, command execution, live CLI behavior, rollout listing, Seatbelt sandboxing, and text encoding. The `core_test_support` helper crate (`@/nori-rs/core/tests/common/`) provides config helpers, macros, and filesystem wait utilities for tests. +The integration test suite in `@/nori-rs/core/tests/suite` covers auth refresh and live CLI behavior; the exec/seatbelt/text-encoding suites now live in `@/nori-rs/sandbox/tests/`. The `core_test_support` helper crate (`@/nori-rs/core/tests/common/`) provides config helpers, macros, and filesystem wait utilities for tests; its exec helper builds shell invocations via `nori_harness::shell` since the shell helpers moved to `@/nori-rs/harness/`, and its sandbox-skip macros use the env-var constants from `codex_sandbox::spawn`. Created and maintained by Nori. diff --git a/nori-rs/core/src/auth.rs b/nori-rs/core/src/auth.rs index d874435e8..63aa33e9b 100644 --- a/nori-rs/core/src/auth.rs +++ b/nori-rs/core/src/auth.rs @@ -24,14 +24,14 @@ use crate::auth::storage::AuthStorageBackend; use crate::auth::storage::create_auth_storage; use crate::config::Config; use crate::default_client::CodexHttpClient; -use crate::error::RefreshTokenFailedError; -use crate::error::RefreshTokenFailedReason; use crate::token_data::KnownPlan as InternalKnownPlan; use crate::token_data::PlanType as InternalPlanType; use crate::token_data::TokenData; use crate::token_data::parse_id_token; use crate::util::try_parse_error_message; use codex_protocol::account::PlanType as AccountPlanType; +use codex_sandbox::error::RefreshTokenFailedError; +use codex_sandbox::error::RefreshTokenFailedReason; use serde_json::Value; use thiserror::Error; diff --git a/nori-rs/core/src/command_safety/is_safe_command.rs b/nori-rs/core/src/command_safety/is_safe_command.rs deleted file mode 100644 index 5e7a7034d..000000000 --- a/nori-rs/core/src/command_safety/is_safe_command.rs +++ /dev/null @@ -1,378 +0,0 @@ -use crate::bash::parse_shell_lc_plain_commands; -use crate::command_safety::windows_safe_commands::is_safe_command_windows; - -pub fn is_known_safe_command(command: &[String]) -> bool { - let command: Vec = command - .iter() - .map(|s| { - if s == "zsh" { - "bash".to_string() - } else { - s.clone() - } - }) - .collect(); - - if is_safe_command_windows(&command) { - return true; - } - - if is_safe_to_call_with_exec(&command) { - return true; - } - - // Support `bash -lc "..."` where the script consists solely of one or - // more "plain" commands (only bare words / quoted strings) combined with - // a conservative allow‑list of shell operators that themselves do not - // introduce side effects ( "&&", "||", ";", and "|" ). If every - // individual command in the script is itself a known‑safe command, then - // the composite expression is considered safe. - if let Some(all_commands) = parse_shell_lc_plain_commands(&command) - && !all_commands.is_empty() - && all_commands - .iter() - .all(|cmd| is_safe_to_call_with_exec(cmd)) - { - return true; - } - false -} - -fn is_safe_to_call_with_exec(command: &[String]) -> bool { - let Some(cmd0) = command.first().map(String::as_str) else { - return false; - }; - - match std::path::Path::new(&cmd0) - .file_name() - .and_then(|osstr| osstr.to_str()) - { - #[rustfmt::skip] - Some( - "cat" | - "cd" | - "echo" | - "false" | - "grep" | - "head" | - "ls" | - "nl" | - "pwd" | - "tail" | - "true" | - "wc" | - "which") => { - true - }, - - Some("find") => { - // Certain options to `find` can delete files, write to files, or - // execute arbitrary commands, so we cannot auto-approve the - // invocation of `find` in such cases. - #[rustfmt::skip] - const UNSAFE_FIND_OPTIONS: &[&str] = &[ - // Options that can execute arbitrary commands. - "-exec", "-execdir", "-ok", "-okdir", - // Option that deletes matching files. - "-delete", - // Options that write pathnames to a file. - "-fls", "-fprint", "-fprint0", "-fprintf", - ]; - - !command - .iter() - .any(|arg| UNSAFE_FIND_OPTIONS.contains(&arg.as_str())) - } - - // Ripgrep - Some("rg") => { - const UNSAFE_RIPGREP_OPTIONS_WITH_ARGS: &[&str] = &[ - // Takes an arbitrary command that is executed for each match. - "--pre", - // Takes a command that can be used to obtain the local hostname. - "--hostname-bin", - ]; - const UNSAFE_RIPGREP_OPTIONS_WITHOUT_ARGS: &[&str] = &[ - // Calls out to other decompression tools, so do not auto-approve - // out of an abundance of caution. - "--search-zip", - "-z", - ]; - - !command.iter().any(|arg| { - UNSAFE_RIPGREP_OPTIONS_WITHOUT_ARGS.contains(&arg.as_str()) - || UNSAFE_RIPGREP_OPTIONS_WITH_ARGS - .iter() - .any(|&opt| arg == opt || arg.starts_with(&format!("{opt}="))) - }) - } - - // Git - Some("git") => matches!( - command.get(1).map(String::as_str), - Some("branch" | "status" | "log" | "diff" | "show") - ), - - // Rust - Some("cargo") if command.get(1).map(String::as_str) == Some("check") => true, - - // Special-case `sed -n {N|M,N}p` - Some("sed") - if { - command.len() <= 4 - && command.get(1).map(String::as_str) == Some("-n") - && is_valid_sed_n_arg(command.get(2).map(String::as_str)) - } => - { - true - } - - // ── anything else ───────────────────────────────────────────────── - _ => false, - } -} - -// (bash parsing helpers implemented in crate::bash) - -/* ---------------------------------------------------------- -Example ----------------------------------------------------------- */ - -/// Returns true if `arg` matches /^(\d+,)?\d+p$/ -fn is_valid_sed_n_arg(arg: Option<&str>) -> bool { - // unwrap or bail - let s = match arg { - Some(s) => s, - None => return false, - }; - - // must end with 'p', strip it - let core = match s.strip_suffix('p') { - Some(rest) => rest, - None => return false, - }; - - // split on ',' and ensure 1 or 2 numeric parts - let parts: Vec<&str> = core.split(',').collect(); - match parts.as_slice() { - // single number, e.g. "10" - [num] => !num.is_empty() && num.chars().all(|c| c.is_ascii_digit()), - - // two numbers, e.g. "1,5" - [a, b] => { - !a.is_empty() - && !b.is_empty() - && a.chars().all(|c| c.is_ascii_digit()) - && b.chars().all(|c| c.is_ascii_digit()) - } - - // anything else (more than one comma) is invalid - _ => false, - } -} - -#[cfg(test)] -mod tests { - use super::*; - use std::string::ToString; - - fn vec_str(args: &[&str]) -> Vec { - args.iter().map(ToString::to_string).collect() - } - - #[test] - fn known_safe_examples() { - assert!(is_safe_to_call_with_exec(&vec_str(&["ls"]))); - assert!(is_safe_to_call_with_exec(&vec_str(&["git", "status"]))); - assert!(is_safe_to_call_with_exec(&vec_str(&[ - "sed", "-n", "1,5p", "file.txt" - ]))); - assert!(is_safe_to_call_with_exec(&vec_str(&[ - "nl", - "-nrz", - "Cargo.toml" - ]))); - - // Safe `find` command (no unsafe options). - assert!(is_safe_to_call_with_exec(&vec_str(&[ - "find", ".", "-name", "file.txt" - ]))); - } - - #[test] - fn zsh_lc_safe_command_sequence() { - assert!(is_known_safe_command(&vec_str(&["zsh", "-lc", "ls"]))); - } - - #[test] - fn unknown_or_partial() { - assert!(!is_safe_to_call_with_exec(&vec_str(&["foo"]))); - assert!(!is_safe_to_call_with_exec(&vec_str(&["git", "fetch"]))); - assert!(!is_safe_to_call_with_exec(&vec_str(&[ - "sed", "-n", "xp", "file.txt" - ]))); - - // Unsafe `find` commands. - for args in [ - vec_str(&["find", ".", "-name", "file.txt", "-exec", "rm", "{}", ";"]), - vec_str(&[ - "find", ".", "-name", "*.py", "-execdir", "python3", "{}", ";", - ]), - vec_str(&["find", ".", "-name", "file.txt", "-ok", "rm", "{}", ";"]), - vec_str(&["find", ".", "-name", "*.py", "-okdir", "python3", "{}", ";"]), - vec_str(&["find", ".", "-delete", "-name", "file.txt"]), - vec_str(&["find", ".", "-fls", "/etc/passwd"]), - vec_str(&["find", ".", "-fprint", "/etc/passwd"]), - vec_str(&["find", ".", "-fprint0", "/etc/passwd"]), - vec_str(&["find", ".", "-fprintf", "/root/suid.txt", "%#m %u %p\n"]), - ] { - assert!( - !is_safe_to_call_with_exec(&args), - "expected {args:?} to be unsafe" - ); - } - } - - #[test] - fn ripgrep_rules() { - // Safe ripgrep invocations – none of the unsafe flags are present. - assert!(is_safe_to_call_with_exec(&vec_str(&[ - "rg", - "Cargo.toml", - "-n" - ]))); - - // Unsafe flags that do not take an argument (present verbatim). - for args in [ - vec_str(&["rg", "--search-zip", "files"]), - vec_str(&["rg", "-z", "files"]), - ] { - assert!( - !is_safe_to_call_with_exec(&args), - "expected {args:?} to be considered unsafe due to zip-search flag", - ); - } - - // Unsafe flags that expect a value, provided in both split and = forms. - for args in [ - vec_str(&["rg", "--pre", "pwned", "files"]), - vec_str(&["rg", "--pre=pwned", "files"]), - vec_str(&["rg", "--hostname-bin", "pwned", "files"]), - vec_str(&["rg", "--hostname-bin=pwned", "files"]), - ] { - assert!( - !is_safe_to_call_with_exec(&args), - "expected {args:?} to be considered unsafe due to external-command flag", - ); - } - } - - #[test] - fn windows_powershell_full_path_is_safe() { - if !cfg!(windows) { - // Windows only because on Linux path splitting doesn't handle `/` separators properly - return; - } - - assert!(is_known_safe_command(&vec_str(&[ - r"C:\Program Files\PowerShell\7\pwsh.exe", - "-Command", - "Get-Location", - ]))); - } - - #[test] - fn bash_lc_safe_examples() { - assert!(is_known_safe_command(&vec_str(&["bash", "-lc", "ls"]))); - assert!(is_known_safe_command(&vec_str(&["bash", "-lc", "ls -1"]))); - assert!(is_known_safe_command(&vec_str(&[ - "bash", - "-lc", - "git status" - ]))); - assert!(is_known_safe_command(&vec_str(&[ - "bash", - "-lc", - "grep -R \"Cargo.toml\" -n" - ]))); - assert!(is_known_safe_command(&vec_str(&[ - "bash", - "-lc", - "sed -n 1,5p file.txt" - ]))); - assert!(is_known_safe_command(&vec_str(&[ - "bash", - "-lc", - "sed -n '1,5p' file.txt" - ]))); - - assert!(is_known_safe_command(&vec_str(&[ - "bash", - "-lc", - "find . -name file.txt" - ]))); - } - - #[test] - fn bash_lc_safe_examples_with_operators() { - assert!(is_known_safe_command(&vec_str(&[ - "bash", - "-lc", - "grep -R \"Cargo.toml\" -n || true" - ]))); - assert!(is_known_safe_command(&vec_str(&[ - "bash", - "-lc", - "ls && pwd" - ]))); - assert!(is_known_safe_command(&vec_str(&[ - "bash", - "-lc", - "echo 'hi' ; ls" - ]))); - assert!(is_known_safe_command(&vec_str(&[ - "bash", - "-lc", - "ls | wc -l" - ]))); - } - - #[test] - fn bash_lc_unsafe_examples() { - assert!( - !is_known_safe_command(&vec_str(&["bash", "-lc", "git", "status"])), - "Four arg version is not known to be safe." - ); - assert!( - !is_known_safe_command(&vec_str(&["bash", "-lc", "'git status'"])), - "The extra quoting around 'git status' makes it a program named 'git status' and is therefore unsafe." - ); - - assert!( - !is_known_safe_command(&vec_str(&["bash", "-lc", "find . -name file.txt -delete"])), - "Unsafe find option should not be auto-approved." - ); - - // Disallowed because of unsafe command in sequence. - assert!( - !is_known_safe_command(&vec_str(&["bash", "-lc", "ls && rm -rf /"])), - "Sequence containing unsafe command must be rejected" - ); - - // Disallowed because of parentheses / subshell. - assert!( - !is_known_safe_command(&vec_str(&["bash", "-lc", "(ls)"])), - "Parentheses (subshell) are not provably safe with the current parser" - ); - assert!( - !is_known_safe_command(&vec_str(&["bash", "-lc", "ls || (pwd && echo hi)"])), - "Nested parentheses are not provably safe with the current parser" - ); - - // Disallowed redirection. - assert!( - !is_known_safe_command(&vec_str(&["bash", "-lc", "ls > out.txt"])), - "> redirection should be rejected" - ); - } -} diff --git a/nori-rs/core/src/command_safety/mod.rs b/nori-rs/core/src/command_safety/mod.rs deleted file mode 100644 index 7d06418ab..000000000 --- a/nori-rs/core/src/command_safety/mod.rs +++ /dev/null @@ -1,2 +0,0 @@ -pub mod is_safe_command; -pub mod windows_safe_commands; diff --git a/nori-rs/core/src/command_safety/windows_safe_commands.rs b/nori-rs/core/src/command_safety/windows_safe_commands.rs deleted file mode 100644 index a1d3b297f..000000000 --- a/nori-rs/core/src/command_safety/windows_safe_commands.rs +++ /dev/null @@ -1,459 +0,0 @@ -use shlex::split as shlex_split; -use std::path::Path; - -/// On Windows, we conservatively allow only clearly read-only PowerShell invocations -/// that match a small safelist. Anything else (including direct CMD commands) is unsafe. -pub fn is_safe_command_windows(command: &[String]) -> bool { - if let Some(commands) = try_parse_powershell_command_sequence(command) { - return commands - .iter() - .all(|cmd| is_safe_powershell_command(cmd.as_slice())); - } - // Only PowerShell invocations are allowed on Windows for now; anything else is unsafe. - false -} - -/// Returns each command sequence if the invocation starts with a PowerShell binary. -/// For example, the tokens from `pwsh Get-ChildItem | Measure-Object` become two sequences. -fn try_parse_powershell_command_sequence(command: &[String]) -> Option>> { - let (exe, rest) = command.split_first()?; - if !is_powershell_executable(exe) { - return None; - } - parse_powershell_invocation(rest) -} - -/// Parses a PowerShell invocation into discrete command vectors, rejecting unsafe patterns. -fn parse_powershell_invocation(args: &[String]) -> Option>> { - if args.is_empty() { - // Examples rejected here: "pwsh" and "powershell.exe" with no additional arguments. - return None; - } - - let mut idx = 0; - while idx < args.len() { - let arg = &args[idx]; - let lower = arg.to_ascii_lowercase(); - match lower.as_str() { - "-command" | "/command" | "-c" => { - let script = args.get(idx + 1)?; - if idx + 2 != args.len() { - // Reject if there is more than one token representing the actual command. - // Examples rejected here: "pwsh -Command foo bar" and "powershell -c ls extra". - return None; - } - return parse_powershell_script(script); - } - _ if lower.starts_with("-command:") || lower.starts_with("/command:") => { - if idx + 1 != args.len() { - // Reject if there are more tokens after the command itself. - // Examples rejected here: "pwsh -Command:dir C:\\" and "powershell /Command:dir C:\\" with trailing args. - return None; - } - let script = arg.split_once(':')?.1; - return parse_powershell_script(script); - } - - // Benign, no-arg flags we tolerate. - "-nologo" | "-noprofile" | "-noninteractive" | "-mta" | "-sta" => { - idx += 1; - continue; - } - - // Explicitly forbidden/opaque or unnecessary for read-only operations. - "-encodedcommand" | "-ec" | "-file" | "/file" | "-windowstyle" | "-executionpolicy" - | "-workingdirectory" => { - // Examples rejected here: "pwsh -EncodedCommand ..." and "powershell -File script.ps1". - return None; - } - - // Unknown switch → bail conservatively. - _ if lower.starts_with('-') => { - // Examples rejected here: "pwsh -UnknownFlag" and "powershell -foo bar". - return None; - } - - // If we hit non-flag tokens, treat the remainder as a command sequence. - // This happens if powershell is invoked without -Command, e.g. - // ["pwsh", "-NoLogo", "git", "-c", "core.pager=cat", "status"] - _ => { - return split_into_commands(args[idx..].to_vec()); - } - } - } - - // Examples rejected here: "pwsh" and "powershell.exe -NoLogo" without a script. - None -} - -/// Tokenizes an inline PowerShell script and delegates to the command splitter. -/// Examples of when this is called: pwsh.exe -Command '