Skip to content

Commit 275a02f

Browse files
madebyarismadebyaris
committed
Enhance error handling and testing discipline in the CLI
- Updated AGENTS.md to clarify that stream/parse failures must propagate typed errors to users instead of appearing as model text. - Added a `schema_version` field to session and event envelopes for forward compatibility. - Introduced a new test discipline in `.cursor/rules/test-discipline.mdc`, mandating regression tests for bug fixes and emphasizing the importance of mocking network access in unit tests. - Updated CI workflows to ensure comprehensive testing and linting, including integration of `cargo-nextest` for improved test execution. - Added new dependencies in Cargo.lock for enhanced functionality and performance. - Introduced a `deny.toml` configuration for managing vulnerabilities and license compliance in dependencies.
1 parent d9e13f8 commit 275a02f

154 files changed

Lines changed: 18383 additions & 8065 deletions

File tree

Some content is hidden

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

‎.config/nextest.toml‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# cargo-nextest configuration
2+
# Docs: https://nexte.st/book/configuration.html
3+
4+
[profile.default]
5+
# Fail fast locally, but still surface a useful summary.
6+
fail-fast = false
7+
slow-timeout = { period = "60s", terminate-after = 3 }
8+
failure-output = "immediate-final"
9+
success-output = "never"
10+
status-level = "pass"
11+
final-status-level = "fail"
12+
13+
# CI profile with retries for flaky network/IPC tests + JUnit output.
14+
[profile.ci]
15+
fail-fast = false
16+
slow-timeout = { period = "120s", terminate-after = 2 }
17+
retries = { backoff = "exponential", count = 2, delay = "2s", max-delay = "20s" }
18+
failure-output = "immediate-final"
19+
success-output = "never"
20+
status-level = "pass"
21+
final-status-level = "slow"
22+
23+
[profile.ci.junit]
24+
path = "junit.xml"
25+
report-name = "nca-nextest"
26+
store-success-output = false
27+
store-failure-output = true
28+
29+
# Integration-test binaries (they spawn the `nca` process, touch the
30+
# filesystem, and hit Unix sockets) run serially so they don't trample each
31+
# other's temp workspaces.
32+
[[profile.default.overrides]]
33+
filter = "binary(cli_commands) or test(integration_)"
34+
test-group = "integration"
35+
36+
[[profile.ci.overrides]]
37+
filter = "binary(cli_commands) or test(integration_)"
38+
test-group = "integration"
39+
40+
[test-groups]
41+
integration = { max-threads = 1 }

‎.cursor/rules/test-discipline.mdc‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
---
2+
description: TDD methodology and quality gate for every behavior change in nca.
3+
globs: "**/*.rs,**/*.snap"
4+
alwaysApply: false
5+
---
6+
7+
# Test Discipline (TDD)
8+
9+
Apply this workflow to every bug fix, refactor, and new behavior in tracks A–C.
10+
11+
## Red → Green → Refactor
12+
13+
1. **Write a failing test first** — unit, integration, or snapshot — that proves the bug or specifies the new behavior.
14+
2. **Land the smallest change** that makes the test pass.
15+
3. **Run the full quality gate** before moving on:
16+
- `cargo nextest run --workspace --features semantic-index`
17+
- `cargo test --workspace --doc`
18+
- `cargo clippy --workspace --all-targets -- -D warnings`
19+
- `cargo fmt --check --all`
20+
- `INSTA_UPDATE=never cargo insta test --check` (when snapshots are involved)
21+
4. **Update supply-chain baselines** if dependencies change (`cargo deny check`, `cargo audit`).
22+
5. **Refactor under green** — re-run the full gate before the next item.
23+
24+
## Test pyramid targets (per crate)
25+
26+
| Crate | Focus |
27+
|-------|-------|
28+
| `nca-common` | Unit + serde snapshot for every `AgentEvent` variant; round-trip session JSON; schema version forward-compat |
29+
| `nca-core` | Unit per provider stream parser; agent-loop integration with mock `Provider`; per-tool unit tests |
30+
| `nca-runtime` | IPC handshake, multi-client broadcast, approve/deny round-trip, cancel-during-stream, worktree lifecycle |
31+
| `nca-tui` | `TuiCmd → SessionRuntime → bridge → state` integration; UiOverlay FSM transitions; composer/onboarding scenarios |
32+
| `nca-cli` | Extend `cli_commands.rs` for `--stream human/ndjson/off`, `--json` parity, live IPC attach |
33+
| `nca-index` | Feature-gated tempdir rebuild/search; `FeatureDisabled` stub round-trip |
34+
35+
## Rules
36+
37+
- **Bug fixes require a regression test** that would have caught the bug.
38+
- **Never depend on network access** in unit tests — mock the `Provider` trait.
39+
- **Use `insta` for rendered output** (TUI transcripts, provider request bodies, event envelopes). Snapshots are locked in CI (`INSTA_UPDATE=never`).
40+
- **Use `tempfile`** for any test that touches the filesystem.
41+
- **Empty provider completions must fail loudly** — assert `AgentEvent::Error`, never silent success.
42+
- **Stream/parse failures must not masquerade as model text** — assert typed errors propagate to the user.
43+
44+
## Integration test isolation
45+
46+
Integration tests (`binary(cli_commands)` or `test(integration_)`) run serially (see `.config/nextest.toml`) because they spawn `nca`, touch the filesystem, and use Unix sockets.

‎.github/workflows/ci.yml‎

Lines changed: 208 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,13 @@ permissions:
1111

1212
env:
1313
CARGO_TERM_COLOR: always
14+
CARGO_INCREMENTAL: "0"
15+
RUST_BACKTRACE: "1"
16+
INSTA_UPDATE: never
1417

1518
jobs:
1619
check:
20+
name: Lint, build, unit + integration tests
1721
runs-on: ubuntu-latest
1822
steps:
1923
- uses: actions/checkout@v4
@@ -33,10 +37,210 @@ jobs:
3337
run: cargo fmt --check --all
3438

3539
- name: Clippy
36-
run: cargo clippy --workspace -- -D warnings
40+
run: cargo clippy --workspace --all-targets -- -D warnings
3741

3842
- name: Build
39-
run: cargo build --workspace
43+
run: cargo build --workspace --all-targets
4044

41-
- name: Test
42-
run: cargo test --workspace
45+
- name: Install nextest
46+
uses: taiki-e/install-action@nextest
47+
48+
- name: Unit + lib tests (nextest, ci profile, excl. integration)
49+
run: cargo nextest run --profile ci --workspace -E 'not (binary(cli_commands) or test(integration_))'
50+
51+
- name: Integration tests (nextest, ci profile, serial)
52+
run: cargo nextest run --profile ci --workspace -E 'binary(cli_commands) or test(integration_)'
53+
54+
- name: Doctests
55+
run: cargo test --workspace --doc
56+
57+
- name: Install cargo-insta
58+
uses: taiki-e/install-action@v2
59+
with:
60+
tool: cargo-insta
61+
62+
- name: Snapshot tests (insta)
63+
run: cargo insta test --check
64+
65+
- name: Upload junit report
66+
if: always()
67+
uses: actions/upload-artifact@v4
68+
with:
69+
name: nextest-junit
70+
path: target/nextest/ci/junit.xml
71+
if-no-files-found: ignore
72+
73+
semantic-index:
74+
name: Tests (semantic-index feature)
75+
runs-on: ubuntu-latest
76+
steps:
77+
- uses: actions/checkout@v4
78+
79+
- uses: dtolnay/rust-toolchain@stable
80+
81+
- uses: Swatinem/rust-cache@v2
82+
83+
- name: Install system dependencies
84+
run: |
85+
sudo apt-get update
86+
sudo apt-get install -y libssl-dev pkg-config ripgrep
87+
88+
- name: Install nextest
89+
uses: taiki-e/install-action@nextest
90+
91+
- name: Build (semantic-index)
92+
run: cargo build --workspace --all-targets --features semantic-index
93+
94+
- name: Unit + lib tests (semantic-index, excl. integration)
95+
run: cargo nextest run --profile ci --workspace --features semantic-index -E 'not (binary(cli_commands) or test(integration_))'
96+
97+
- name: Integration tests (semantic-index, serial)
98+
run: cargo nextest run --profile ci --workspace --features semantic-index -E 'binary(cli_commands) or test(integration_)'
99+
100+
msrv:
101+
name: MSRV check (Rust 1.85)
102+
runs-on: ubuntu-latest
103+
steps:
104+
- uses: actions/checkout@v4
105+
106+
- uses: dtolnay/rust-toolchain@master
107+
with:
108+
toolchain: "1.85"
109+
components: rustfmt, clippy
110+
111+
- uses: Swatinem/rust-cache@v2
112+
113+
- name: Install system dependencies
114+
run: |
115+
sudo apt-get update
116+
sudo apt-get install -y libssl-dev pkg-config ripgrep
117+
118+
- name: Check formatting
119+
run: cargo fmt --check --all
120+
121+
- name: Clippy
122+
run: cargo clippy --workspace --all-targets -- -D warnings
123+
124+
- name: Install nextest
125+
uses: taiki-e/install-action@nextest
126+
127+
- name: Unit + lib tests (MSRV, excl. integration)
128+
run: cargo nextest run --profile ci --workspace -E 'not (binary(cli_commands) or test(integration_))'
129+
130+
- name: Integration tests (MSRV, serial)
131+
run: cargo nextest run --profile ci --workspace -E 'binary(cli_commands) or test(integration_)'
132+
133+
check-macos:
134+
name: Tests (macOS)
135+
runs-on: macos-14
136+
steps:
137+
- uses: actions/checkout@v4
138+
139+
- uses: dtolnay/rust-toolchain@stable
140+
141+
- uses: Swatinem/rust-cache@v2
142+
143+
- name: Install ripgrep
144+
run: brew install ripgrep
145+
146+
- name: Install nextest
147+
uses: taiki-e/install-action@nextest
148+
149+
- name: Build
150+
run: cargo build --workspace --all-targets
151+
152+
- name: Unit + lib tests (macOS, excl. integration)
153+
run: cargo nextest run --profile ci --workspace -E 'not (binary(cli_commands) or test(integration_))'
154+
155+
- name: Integration tests (macOS, serial)
156+
run: cargo nextest run --profile ci --workspace -E 'binary(cli_commands) or test(integration_)'
157+
158+
bench-smoke:
159+
name: Bench compile smoke
160+
runs-on: ubuntu-latest
161+
needs: check
162+
steps:
163+
- uses: actions/checkout@v4
164+
165+
- uses: dtolnay/rust-toolchain@stable
166+
167+
- uses: Swatinem/rust-cache@v2
168+
169+
- name: Install system dependencies
170+
run: |
171+
sudo apt-get update
172+
sudo apt-get install -y libssl-dev pkg-config
173+
174+
- name: Compile Criterion benches (no run)
175+
run: |
176+
cargo bench -p nca-runtime --bench session_store_load --no-run
177+
cargo bench -p nca-tui --bench tui_text --no-run
178+
179+
- name: Bench regression note
180+
run: |
181+
echo "Criterion baselines are recorded manually in docs/research/baselines.md."
182+
echo "Compare locally with: cargo bench -- --baseline main"
183+
184+
bloat:
185+
name: Binary size (cargo-bloat)
186+
runs-on: ubuntu-latest
187+
needs: check
188+
steps:
189+
- uses: actions/checkout@v4
190+
- uses: dtolnay/rust-toolchain@stable
191+
- uses: Swatinem/rust-cache@v2
192+
193+
- name: Install system dependencies
194+
run: |
195+
sudo apt-get update
196+
sudo apt-get install -y libssl-dev pkg-config
197+
198+
- name: Install cargo-bloat
199+
run: cargo install cargo-bloat --locked
200+
201+
- name: Build release nca
202+
run: cargo build --release -p nca-cli
203+
204+
- name: Report top crates by size
205+
run: cargo bloat --release -p nca-cli --crates -n 30 || true
206+
207+
- name: Report top functions by size
208+
run: cargo bloat --release -p nca-cli -n 30 || true
209+
210+
- name: Report release-small binary size
211+
run: |
212+
cargo build --profile release-small -p nca-cli
213+
ls -la target/release-small/nca
214+
size target/release-small/nca || true
215+
216+
audit:
217+
name: Security audit (cargo audit)
218+
runs-on: ubuntu-latest
219+
steps:
220+
- uses: actions/checkout@v4
221+
- uses: dtolnay/rust-toolchain@stable
222+
- uses: Swatinem/rust-cache@v2
223+
224+
- name: Install cargo-audit
225+
uses: taiki-e/install-action@v2
226+
with:
227+
tool: cargo-audit
228+
229+
- name: cargo audit
230+
run: cargo audit --deny warnings
231+
232+
deny:
233+
name: Supply chain (cargo deny)
234+
runs-on: ubuntu-latest
235+
steps:
236+
- uses: actions/checkout@v4
237+
- uses: dtolnay/rust-toolchain@stable
238+
- uses: Swatinem/rust-cache@v2
239+
240+
- name: Install cargo-deny
241+
uses: taiki-e/install-action@v2
242+
with:
243+
tool: cargo-deny
244+
245+
- name: cargo deny check
246+
run: cargo deny --all-features check

‎.github/workflows/release.yml‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ permissions:
1616
env:
1717
CARGO_TERM_COLOR: always
1818
BINARY_NAME: nca
19+
INSTA_UPDATE: never
1920

2021
jobs:
2122
# Determine if this is a release-worthy push
@@ -63,10 +64,27 @@ jobs:
6364
run: cargo fmt --check --all
6465

6566
- name: Clippy
66-
run: cargo clippy --workspace -- -D warnings
67+
run: cargo clippy --workspace --all-targets -- -D warnings
6768

68-
- name: Test
69-
run: cargo test --workspace
69+
- name: Install nextest
70+
uses: taiki-e/install-action@nextest
71+
72+
- name: Unit + lib tests (nextest, ci profile, excl. integration)
73+
run: cargo nextest run --profile ci --workspace -E 'not (binary(cli_commands) or test(integration_))'
74+
75+
- name: Integration tests (nextest, ci profile, serial)
76+
run: cargo nextest run --profile ci --workspace -E 'binary(cli_commands) or test(integration_)'
77+
78+
- name: Doctests
79+
run: cargo test --workspace --doc
80+
81+
- name: Install cargo-insta
82+
uses: taiki-e/install-action@v2
83+
with:
84+
tool: cargo-insta
85+
86+
- name: Snapshot tests (insta)
87+
run: cargo insta test --check
7088

7189
# Multi-platform build
7290
build:

‎AGENTS.md‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
- MiniMax M2.5 is the primary LLM provider; prioritize MiniMax integration over other providers
55
- **CLI (`nca`) is the product surface**—terminal UX, JSON/NDJSON streams, and Unix-socket IPC
66
- Empty provider completions must fail loudly, never silently succeed
7+
- Stream/parse failures must not masquerade as model text — propagate typed errors to the user
78
- Use plan-first workflow: create a plan document, then implement from it
89
- Install CLI via `cargo build --release` then `cp target/release/nca /usr/local/bin/`
910
- Keep each crate focused: `common` for shared types, `core` for agent logic, `runtime` for session lifecycle, `cli` for terminal UX
@@ -13,14 +14,26 @@
1314

1415
## Learned Workspace Facts
1516

16-
- Rust workspace with **4 crates**: `nca-common`, `nca-core`, `nca-runtime`, `nca-cli`
17+
- Rust workspace with **5 crates**: `nca-common`, `nca-core`, `nca-runtime`, `nca-tui`, `nca-cli`
1718
- IPC between CLI and runtime uses Unix domain sockets with newline-delimited JSON
1819
- Sessions persisted as `<id>.json` (state) + `<id>.events.jsonl` (event log) in `<workspace>/.nca/sessions/`
20+
- Session and event envelopes include `schema_version` (currently `1`) for forward-compatible loading
1921
- MiniMax provider endpoint: `https://api.minimaxi.chat/v1/text/chatcompletion_v2`
22+
- Provider request bodies for MiniMax/OpenAI/Anthropic are snapshot-tested in `crates/core/tests/provider_request_bodies.rs` (`INSTA_UPDATE=never` in CI)
2023
- Global config at `~/.nca/config.toml`
2124
- Git worktrees for isolated agent runs stored at `<repo>/.nca/worktrees/<session-id>`
2225
- **Single shipped app binary:** `nca` (CLI)
2326
- Tokio async runtime; `async-trait` for tool executor and approval handler interfaces
2427
- Session lineage: parent/child session IDs, inherited summary, spawn reason tracked in `SessionMeta`
2528
- `AgentEvent` enum is the shared event bus for CLI rendering, IPC broadcast, and disk persistence
2629
- Runtime socket dir defaults to `$XDG_RUNTIME_DIR/nca/` or `/tmp/nca/`
30+
- Cooperative cancel: IPC `AgentCommand::Cancel` → `CancellationToken` in the agent loop; session state is persisted on finish
31+
- Bash tools run in a real PTY via `portable-pty`, not piped `std::process::Command`
32+
33+
## Test discipline
34+
35+
- **Bug fixes require a regression test** that would have caught the bug (see `.cursor/rules/test-discipline.mdc`)
36+
- Run the full gate before marking work done: `cargo nextest run --workspace --features semantic-index`, `cargo clippy --workspace --all-targets -- -D warnings`, `cargo fmt --check --all`
37+
- Never depend on network access in unit tests — mock the `Provider` trait
38+
- Empty provider completions: assert `AgentEvent::Error`, never silent success
39+
- Stream/parse failures: assert typed errors propagate, never model text

0 commit comments

Comments
 (0)