diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..e326517 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,21 @@ +name: CI + +on: + push: + pull_request: + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + with: + components: clippy + - name: Test + run: cargo test --all-targets --no-fail-fast + - name: Lint + run: cargo clippy --all-targets -- -D warnings diff --git a/.gitignore b/.gitignore index 2dca71b..9a2912e 100644 --- a/.gitignore +++ b/.gitignore @@ -24,8 +24,12 @@ target # Test binaries tests/e2e/test_* tests/fixtures/test_* +tests/fixtures/**/test_* tests/fixtures/cuda_test +# Plan files (temporary) +.plan/ + # JS/TS stuff node_modules/ package-lock.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 748e9c1..e3a19e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,49 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- Debuggee output reads are now cursor-based and non-destructive. `GetOutput` + accepts an optional `cursor` and returns `next_cursor`, so concurrent + `output --follow` clients and one-shot `output` calls each see the full + stream; old data expires only via the buffer's size bounds or an explicit + `--clear`. +- `DebugSession` was split internally: a reducer-owned `SessionModel` holds + lifecycle/thread/frame/breakpoint/exit state, and a cursor-based + `OutputBuffer` holds debuggee output, with `DebugSession` remaining the + façade over the DAP transport. All DAP requests now go through a single + send path, so normal and deferred-response requests (GDB/debugpy `launch`) + share one lifecycle. +- The daemon now handles client connections concurrently. A session actor owns + the debug session and serializes DAP requests, while `await` waits on state + snapshots — so `pause`, `status`, and other commands from a second terminal + work while another client is blocked in `await`. DAP events are reduced on a + 100ms tick instead of a 1-second tick that paused while a client was + connected. +- `output --follow` keeps one connection open instead of reconnecting for each + poll, which is safe now that connections no longer block each other. + +### Added + +- A non-ignored native GDB DAP integration test for startup breakpoints, + selected-frame context, expression evaluation, output capture, and output + tailing when GDB 14.1+ is available. +- GitHub Actions coverage for the Rust test suite and Clippy. +- `break --hit-count ` parity with `breakpoint add --hit-count `. + +### Fixed + +- DAP launch sequencing for adapters that defer their `launch` response until + after `configurationDone`, including native GDB and debugpy. +- `output --follow`, output clearing byte accounting, UTF-8-safe buffer limits, + and line-based `output --tail` behavior. +- Selected frames are preserved by `context`, and thread selection refreshes + adapter threads before accepting an ID. +- Initial breakpoints are tracked for later list/remove/enable operations; + breakpoint state rolls back if an adapter request fails. + ## [0.1.1] - 2026-01-25 ### Added diff --git a/Cargo.lock b/Cargo.lock index 91e4b04..ebf4d51 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -365,7 +365,7 @@ dependencies = [ [[package]] name = "debugger-cli" -version = "0.1.1" +version = "0.1.3" dependencies = [ "async-trait", "clap", diff --git a/Cargo.toml b/Cargo.toml index a883cda..2357050 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "debugger-cli" -version = "0.1.1" +version = "0.1.3" edition = "2021" description = "LLM-friendly debugger CLI using the Debug Adapter Protocol" license = "GPL-3.0-only" diff --git a/PROGRESS.md b/PROGRESS.md index d26ddbf..ba1da98 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -2,9 +2,9 @@ This document tracks the implementation status of debugger-cli. -> **Current Version**: 0.1.1 +> **Current Version**: 0.1.3 > **Status**: Feature-complete for core debugging workflows -> **Last Updated**: 2026-01-25 +> **Last Updated**: 2026-07-09 ## Implementation Status @@ -50,7 +50,7 @@ This document tracks the implementation status of debugger-cli. | `stop` | | ✅ | Stop debug session | | `detach` | | ✅ | Detach (keep process running) | | `status` | | ✅ | Show daemon/session status | -| `restart` | | ✅ | Restart program with same args | +| `restart` | | ✅ | Restart when the active adapter supports the DAP restart request | | `breakpoint add` | `break`, `b` | ✅ | Add breakpoint | | `breakpoint remove` | | ✅ | Remove breakpoint | | `breakpoint list` | | ✅ | List all breakpoints | diff --git a/README.md b/README.md index 6ab1fe2..2dd100b 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,7 @@ debugger stop | `stop` | | Stop debug session and terminate debuggee | | `detach` | | Detach from process (keeps it running) | | `status` | | Show daemon and session status | -| `restart` | | Restart program with same arguments | +| `restart` | | Restart program when supported by the active DAP adapter | Start options: - `--adapter ` - Use specific debug adapter @@ -126,6 +126,8 @@ Start options: | `breakpoint remove ` | | Remove breakpoint by ID | | `breakpoint remove --all` | | Remove all breakpoints | | `breakpoint list` | | List all breakpoints | +| `breakpoint enable ` | | Enable a disabled breakpoint | +| `breakpoint disable ` | | Disable a breakpoint without removing it | Breakpoint options: - `--condition ` - Break only when expression is true @@ -169,6 +171,7 @@ Breakpoint options: | `output` | Get program stdout/stderr | | `output --follow` | Stream output continuously | | `output --tail ` | Get last N lines | +| `output --clear` | Print and clear buffered output | ### Setup @@ -226,7 +229,7 @@ codelldb = "~/.local/share/debugger-cli/adapters/codelldb/adapter/codelldb" | GDB | C, C++ | ✅ Full support (requires GDB 14.1+) | | CUDA-GDB | CUDA, C, C++ | ✅ Full support (Linux only) | | js-debug | JavaScript, TypeScript | ✅ Full support | -| CodeLLDB | C, C++, Rust | 🚧 Planned | +| CodeLLDB | C, C++, Rust | ✅ Full support | | cpptools | C, C++ | 🚧 Planned | ## Examples diff --git a/docs/plan/architecture-review-2026-07.md b/docs/plan/architecture-review-2026-07.md new file mode 100644 index 0000000..cbc62a3 --- /dev/null +++ b/docs/plan/architecture-review-2026-07.md @@ -0,0 +1,135 @@ +# Architecture Review — 2026-07 + +## Current Shape + +`debugger-cli` is one binary in two modes. Short-lived CLI invocations send a +length-prefixed JSON request to a local daemon, which owns one `DebugSession`. +The session owns the DAP adapter process and a background reader task; DAP +events are buffered until the daemon processes them. + +This is the right product-level split. It makes an otherwise interactive DAP +connection usable from independent agent commands, and it prevents output and +stop events from being lost between invocations. + +## What Is Working Well + +- The DAP reader task keeps adapter I/O independent from CLI process lifetime. +- `DebugSession` centralizes adapter capability checks and selected + thread/frame state. +- The IPC boundary keeps CLI presentation code out of DAP mechanics. +- Adapter setup is isolated from the debugging path. + +## Risks and Friction + +1. **The daemon is single-client and serial.** `handle_client` owns the daemon + until that connection closes. A long `await`, or any future true streaming + command, blocks other CLI clients. Concretely: while one terminal runs + `await --timeout 30`, a second terminal cannot send `pause` or `continue` — + the command that would cause the stop is blocked behind the command waiting + for it. The current `output --follow` avoids this by reconnecting for each + poll, but that is a containment measure rather than a general concurrency + model. + + A related defect: DAP events are only reduced on the accept loop's 1-second + tick, and that tick does not run at all while a client connection is open, + because `handle_client` is awaited inline. Handlers that need fresh state + (`output`, `await`) each call `process_events()` manually as a workaround. + +2. **`DebugSession` has too many responsibilities.** It owns DAP transport, + session lifecycle, event reduction, breakpoint persistence, output storage, + selected-view state, and inspection helpers. This makes a feature such as + multi-session support or a richer output API expensive to add safely. + +3. **DAP request lifecycle is split across several paths.** Normal requests, + deferred launch, and fire-and-forget disconnect have subtly different + response handling. Adapters may legally delay `launch` until after + `configurationDone`; this was the root cause of the GDB startup deadlock + fixed in this pass. + +4. **The IPC protocol is typed at the command edge but loosely typed at the + result edge.** Most responses are `serde_json::Value`, so the daemon and CLI + can silently drift in field names and error handling. + +5. **Validation is adapter-sparse.** Unit tests are useful, but many core + workflows were ignored when their adapter was absent. A real GDB smoke test + now runs opportunistically on GDB 14.1+, but the other supported adapters + still need reproducible coverage. + +## Recommended Evolution + +### Phase 1 — Make command handling concurrent + +Keep exactly one debug session, but move it behind a session actor: + +```text +IPC clients ──> request tasks ──> session command channel ──> DebugSession + │ +DAP reader ──> event reducer ─────────────┴──> broadcast/watch state updates +``` + +Each IPC connection can then be handled independently. The actor serializes +DAP requests (which preserves adapter ordering), while `await`, status, and +future subscriptions can wait on state updates without holding the listener. +This is the highest-value architecture change. + +Two constraints on the actor design: + +- The actor must own the session *lifecycle*, not just command dispatch. + `start`/`stop`/`detach` create and destroy the session, so session + creation and teardown belong in the same serialized command domain. +- Decision 3 below (destructive vs. cursor-based output) should be resolved + before this phase, not before a larger rewrite: the actor's broadcast/watch + design bakes in one answer or the other. + +### Phase 2 — Split session state from DAP orchestration + +Extract three focused components: + +- `DapTransport`: request IDs, timeouts, deferred-response handling, and raw + adapter events. +- `SessionModel`: reducer-owned state for lifecycle, threads, frames, + breakpoints, and exit status. +- `OutputBuffer`: bounded output with a monotonic cursor. A cursor lets clients + follow output without clearing data another client has not read. + +`DebugSession` can remain the façade initially, so this is an internal, +incremental refactor rather than a CLI redesign. + +### Phase 3 — Stabilize the public automation contract + +Add a `--json` output mode for every inspection and control command, with +versioned response structs rather than ad-hoc JSON values. Keep human output as +the default. This is especially valuable for LLM agents and scenario tests. + +At the same time, version the local IPC envelope. The Unix socket is already +restricted to the owning user (`0o600` in `transport.rs`); the remaining gap is +the Windows named-pipe security descriptor. The daemon is a local control plane +for arbitrary debuggee processes, so it should never become a casually exposed +network service. + +### Phase 4 — Broaden confidence deliberately + +- Keep the native GDB test as a no-install smoke test where supported. +- Use containers or a scheduled CI matrix for LLDB, debugpy, Delve, and + js-debug rather than claiming an unprovisioned matrix runs on every PR. +- Add mock-DAP tests for request ordering, delayed launch responses, output + cursor semantics, and adapter failures. These cover protocol behavior without + making every unit test depend on a toolchain. + +## Decisions to Make Before a Larger Rewrite + +1. Should one daemon support multiple named sessions, or should session + isolation remain one daemon per target? +2. Is JSON output a supported public contract, or only an internal testing + convenience? +3. Should output follow be destructive (current polling behavior) or + cursor-based and independent per client? + **Resolved 2026-07-09: cursor-based.** `GetOutput` takes an optional + cursor and returns `next_cursor`; reads are non-destructive and + per-client, with `--clear` retained as an explicit destructive op. +4. Do we want to preserve a zero-install default, or accept containerized + adapter CI as a required release gate? + +The recommended default is: one session for now, a versioned JSON contract, +cursor-based non-destructive output, and containerized adapter coverage before +advertising a feature as fully supported. diff --git a/src/cli/mod.rs b/src/cli/mod.rs index a259e7f..6b7dd9b 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -52,8 +52,10 @@ pub async fn dispatch(command: Commands) -> Result<()> { println!("Set {} initial breakpoint(s)", initial_breakpoints.len()); } - if stop_on_entry || has_initial_breakpoints { + if stop_on_entry { println!("Stopped at entry point. Use 'debugger continue' to run."); + } else if has_initial_breakpoints { + println!("Program is running. It will stop when an initial breakpoint is hit."); } else { println!("Program is running. Use 'debugger await' to wait for a stop."); } @@ -150,7 +152,11 @@ pub async fn dispatch(command: Commands) -> Result<()> { } }, - Commands::Break { location, condition } => { + Commands::Break { + location, + condition, + hit_count, + } => { // Shorthand for breakpoint add let mut client = DaemonClient::connect().await?; let loc = BreakpointLocation::parse(&location)?; @@ -159,7 +165,7 @@ pub async fn dispatch(command: Commands) -> Result<()> { .send_command(Command::BreakpointAdd { location: loc, condition, - hit_count: None, + hit_count, }) .await?; @@ -398,7 +404,7 @@ pub async fn dispatch(command: Commands) -> Result<()> { // Show current thread info let result = client.send_command(Command::Status).await?; let status: StatusResult = serde_json::from_value(result)?; - if let Some(thread_id) = status.stopped_thread { + if let Some(thread_id) = status.selected_thread { println!("Current thread: {}", thread_id); } else { println!("No thread selected"); @@ -472,15 +478,39 @@ pub async fn dispatch(command: Commands) -> Result<()> { } Commands::Output { follow, tail, clear } => { - let mut client = DaemonClient::connect().await?; - if follow { - println!("Output streaming not yet implemented"); - return Ok(()); + use std::io::Write; + + eprintln!("Following debuggee output (Ctrl+C to stop)"); + // Connections are handled concurrently by the daemon, so one + // long-lived connection can poll without blocking other clients. + let mut client = DaemonClient::connect().await?; + // Poll from this follower's own cursor: non-destructive, so + // concurrent followers and one-shot `output` calls each see + // the full stream. + let mut cursor: Option = None; + loop { + let result = client + .send_command(Command::GetOutput { + tail: None, + clear: false, + cursor, + }) + .await?; + let output = result["output"].as_str().unwrap_or(""); + if !output.is_empty() { + print!("{}", output); + std::io::stdout().flush()?; + } + cursor = result["next_cursor"].as_u64().or(cursor); + + tokio::time::sleep(std::time::Duration::from_millis(200)).await; + } } + let mut client = DaemonClient::connect().await?; let result = client - .send_command(Command::GetOutput { tail, clear }) + .send_command(Command::GetOutput { tail, clear, cursor: None }) .await?; let output = result["output"].as_str().unwrap_or(""); diff --git a/src/commands.rs b/src/commands.rs index cd161fb..14fec76 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -53,6 +53,10 @@ pub enum Commands { /// Condition for the breakpoint #[arg(long, short)] condition: Option, + + /// Hit count (break after N hits) + #[arg(long)] + hit_count: Option, }, /// Continue execution @@ -141,7 +145,7 @@ pub enum Commands { /// Get debuggee stdout/stderr output Output { /// Stream output continuously - #[arg(long)] + #[arg(long, conflicts_with_all = ["tail", "clear"])] follow: bool, /// Get last N lines of output diff --git a/src/common/config.rs b/src/common/config.rs index f632b61..4f48dba 100644 --- a/src/common/config.rs +++ b/src/common/config.rs @@ -199,19 +199,99 @@ impl Config { /// Get adapter configuration by name /// - /// Falls back to searching PATH if not explicitly configured + /// Falls back to searching PATH if not explicitly configured. + /// For common adapters, also tries alternative names (e.g., lldb-vscode for lldb-dap). pub fn get_adapter(&self, name: &str) -> Option { // Check explicit configuration first if let Some(config) = self.adapters.get(name) { return Some(config.clone()); } - // Try to find in PATH - which::which(name).ok().map(|path| AdapterConfig { - path, - args: Vec::new(), - transport: TransportMode::default(), - spawn_style: TcpSpawnStyle::default(), - }) + // Build list of names to try: primary name + any fallbacks + let names_to_try = adapter_fallback_names(name); + + // Try to find any of the names in PATH + for try_name in &names_to_try { + if let Ok(path) = which::which(try_name) { + return Some(AdapterConfig { + path, + args: Vec::new(), + transport: TransportMode::default(), + spawn_style: TcpSpawnStyle::default(), + }); + } + } + + // For LLDB, also check known system paths (especially for macOS) + if matches!(name, "lldb-dap" | "lldb-vscode" | "lldb") { + for known_path in known_lldb_paths() { + if known_path.exists() { + return Some(AdapterConfig { + path: known_path, + args: Vec::new(), + transport: TransportMode::default(), + spawn_style: TcpSpawnStyle::default(), + }); + } + } + } + + None + } +} + +/// Returns known system paths where lldb-dap might be installed. +/// This is especially useful on macOS where the binary might not be in PATH. +fn known_lldb_paths() -> Vec { + vec![ + // macOS: Xcode command line tools + PathBuf::from("/usr/bin/lldb-dap"), + PathBuf::from("/usr/bin/lldb-vscode"), + // macOS: Xcode internal paths + PathBuf::from("/Applications/Xcode.app/Contents/Developer/usr/bin/lldb-dap"), + PathBuf::from("/Applications/Xcode.app/Contents/Developer/usr/bin/lldb-vscode"), + // macOS: Homebrew LLVM (Intel) + PathBuf::from("/usr/local/opt/llvm/bin/lldb-dap"), + PathBuf::from("/usr/local/opt/llvm/bin/lldb-vscode"), + // macOS: Homebrew LLVM (Apple Silicon) + PathBuf::from("/opt/homebrew/opt/llvm/bin/lldb-dap"), + PathBuf::from("/opt/homebrew/opt/llvm/bin/lldb-vscode"), + // Linux: common locations + PathBuf::from("/usr/lib/llvm-19/bin/lldb-dap"), + PathBuf::from("/usr/lib/llvm-18/bin/lldb-dap"), + PathBuf::from("/usr/lib/llvm-17/bin/lldb-dap"), + PathBuf::from("/usr/lib/llvm-16/bin/lldb-dap"), + PathBuf::from("/usr/lib/llvm-19/bin/lldb-vscode"), + PathBuf::from("/usr/lib/llvm-18/bin/lldb-vscode"), + PathBuf::from("/usr/lib/llvm-17/bin/lldb-vscode"), + PathBuf::from("/usr/lib/llvm-16/bin/lldb-vscode"), + ] +} + +/// Returns a list of adapter names to try, with the primary name first. +/// This handles cases where adapters have different names on different systems +/// (e.g., lldb-dap vs lldb-vscode on Ubuntu, versioned names like lldb-dap-18). +pub fn adapter_fallback_names(name: &str) -> Vec { + match name { + // LLDB adapter: try unversioned first, then versioned variants + // On Ubuntu, apt installs versioned binaries like lldb-dap-18 or lldb-vscode-18 + // LLVM 18+ renamed lldb-vscode to lldb-dap + "lldb-dap" | "lldb-vscode" | "lldb" => vec![ + "lldb-dap".to_string(), + "lldb-vscode".to_string(), + // Versioned variants (newest first) - Ubuntu apt installs these + "lldb-dap-19".to_string(), + "lldb-dap-18".to_string(), + "lldb-dap-17".to_string(), + "lldb-dap-16".to_string(), + "lldb-vscode-19".to_string(), + "lldb-vscode-18".to_string(), + "lldb-vscode-17".to_string(), + "lldb-vscode-16".to_string(), + "lldb-vscode-15".to_string(), + "lldb-vscode-14".to_string(), + ], + // Other adapters just use their exact name + _ => vec![name.to_string()], } } diff --git a/src/common/error.rs b/src/common/error.rs index 1ad5144..f8b7eba 100644 --- a/src/common/error.rs +++ b/src/common/error.rs @@ -117,10 +117,10 @@ pub enum Error { impl Error { /// Create an adapter not found error with search paths - pub fn adapter_not_found(name: &str, paths: &[&str]) -> Self { + pub fn adapter_not_found>(name: &str, paths: &[S]) -> Self { Self::AdapterNotFound { name: name.to_string(), - searched: paths.join(", "), + searched: paths.iter().map(|s| s.as_ref()).collect::>().join(", "), } } diff --git a/src/daemon/actor.rs b/src/daemon/actor.rs new file mode 100644 index 0000000..954e535 --- /dev/null +++ b/src/daemon/actor.rs @@ -0,0 +1,111 @@ +//! Session actor - owns the debug session and serializes access to it +//! +//! Connection tasks send commands over an mpsc channel; the actor executes +//! them one at a time, which preserves DAP request ordering. After every +//! command and on a periodic tick it reduces pending DAP events and publishes +//! a state snapshot on a watch channel, so `await` (and any future +//! subscription) can wait on state changes without occupying the actor. + +use std::sync::Arc; +use std::time::Duration; + +use tokio::sync::{mpsc, oneshot, watch}; + +use crate::common::config::Config; +use crate::dap::StoppedEventBody; +use crate::ipc::protocol::{Command, Response}; + +use super::handler; +use super::session::{DebugSession, SessionState}; + +/// How often the actor reduces DAP events when no commands arrive. +const EVENT_TICK: Duration = Duration::from_millis(100); + +/// A command forwarded from a connection task, with a channel for the reply. +pub struct ActorRequest { + pub id: u64, + pub command: Command, + pub reply: oneshot::Sender, +} + +/// Published view of the session, updated after every event reduction. +#[derive(Debug, Clone, Default, PartialEq)] +pub struct SessionSnapshot { + pub session_active: bool, + pub state: Option, + /// Full stopped-event body, when the stop came from an adapter event. + pub last_stop: Option, + /// Stop reason fallback for stops without an event (attach, stop-on-entry). + pub stopped_reason: Option, + pub stopped_thread: Option, + pub exit_code: Option, +} + +/// Run the session actor until every request sender is dropped. +/// +/// On exit the actor stops any remaining session, so daemon shutdown only +/// needs to drop its sender and await this task. +pub async fn run( + config: Arc, + mut requests: mpsc::Receiver, + snapshots: watch::Sender, +) { + let mut session: Option = None; + let mut tick = tokio::time::interval(EVENT_TICK); + tick.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); + + loop { + tokio::select! { + request = requests.recv() => { + let Some(ActorRequest { id, command, reply }) = request else { + break; + }; + + reduce_events(&mut session).await; + let response = handler::handle_command(&mut session, &config, id, command).await; + publish(&snapshots, &session); + let _ = reply.send(response); + } + _ = tick.tick() => { + reduce_events(&mut session).await; + publish(&snapshots, &session); + } + } + } + + tracing::debug!("Session actor shutting down"); + if let Some(mut active) = session.take() { + let _ = active.stop().await; + } +} + +async fn reduce_events(session: &mut Option) { + if let Some(active) = session.as_mut() { + if let Err(e) = active.process_events().await { + tracing::warn!("Error processing events: {}", e); + } + } +} + +fn publish(snapshots: &watch::Sender, session: &Option) { + let snapshot = match session { + Some(active) => SessionSnapshot { + session_active: true, + state: Some(active.state()), + last_stop: active.last_stop().cloned(), + stopped_reason: active.stopped_reason().map(String::from), + stopped_thread: active.stopped_thread(), + exit_code: active.exit_code(), + }, + None => SessionSnapshot::default(), + }; + + snapshots.send_if_modified(|current| { + if *current == snapshot { + false + } else { + *current = snapshot; + true + } + }); +} diff --git a/src/daemon/handler.rs b/src/daemon/handler.rs index 93dc45d..1adab7c 100644 --- a/src/daemon/handler.rs +++ b/src/daemon/handler.rs @@ -5,32 +5,12 @@ use serde_json::json; use crate::common::{config::Config, error::IpcError, Error, Result}; -use crate::dap::{Event, StackFrame}; use crate::ipc::protocol::{ BreakpointLocation, Command, ContextResult, EvaluateContext, EvaluateResult, Response, - SourceLine, StackFrameInfo, StatusResult, StopResult, ThreadInfo, VariableInfo, + SourceLine, StackFrameInfo, StatusResult, ThreadInfo, VariableInfo, }; -use super::session::{DebugSession, SessionState}; - -/// Extract source location info (filename, line, column) from the top stack frame -fn extract_source_location(frames: &[StackFrame]) -> (Option, Option, Option) { - if frames.is_empty() { - return (None, None, None); - } - - let frame = &frames[0]; - let source_path = frame.source.as_ref().and_then(|s| s.path.clone()); - // Extract just the filename from the path - let source_name = source_path.as_ref().map(|p| { - std::path::Path::new(p) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(p) - .to_string() - }); - (source_name, Some(frame.line as u32), Some(frame.column as u32)) -} +use super::session::DebugSession; /// Handle an IPC command pub async fn handle_command( @@ -126,6 +106,7 @@ async fn handle_command_inner( state: Some(sess.state().to_string()), program: Some(sess.program().display().to_string()), adapter: Some(sess.adapter_name().to_string()), + selected_thread: sess.get_selected_thread(), stopped_thread: sess.stopped_thread(), stopped_reason: sess.stopped_reason().map(String::from), } @@ -136,6 +117,7 @@ async fn handle_command_inner( state: None, program: None, adapter: None, + selected_thread: None, stopped_thread: None, stopped_reason: None, } @@ -153,12 +135,13 @@ async fn handle_command_inner( let sess = session.as_mut().ok_or(Error::SessionNotActive)?; // Check capabilities before using advanced features - if matches!(location, BreakpointLocation::Function { .. }) { - if !sess.supports_function_breakpoints() { - return Err(Error::Internal( - "Debug adapter does not support function breakpoints. Use file:line format instead.".to_string() - )); - } + if matches!(location, BreakpointLocation::Function { .. }) + && !sess.supports_function_breakpoints() + { + return Err(Error::Internal( + "Debug adapter does not support function breakpoints. Use file:line format instead." + .to_string(), + )); } if condition.is_some() && !sess.supports_conditional_breakpoints() { @@ -243,9 +226,9 @@ async fn handle_command_inner( } // === State Inspection === - Command::StackTrace { thread_id: _, limit } => { + Command::StackTrace { thread_id, limit } => { let sess = session.as_mut().ok_or(Error::SessionNotActive)?; - let frames = sess.stack_trace(limit).await?; + let frames = sess.stack_trace(thread_id, limit).await?; let frame_infos: Vec = frames .iter() @@ -340,7 +323,7 @@ async fn handle_command_inner( Command::ThreadSelect { id } => { let sess = session.as_mut().ok_or(Error::SessionNotActive)?; - sess.select_thread(id)?; + sess.select_thread(id).await?; Ok(json!({ "selected": id })) } @@ -368,11 +351,9 @@ async fn handle_command_inner( Command::Context { lines } => { let sess = session.as_mut().ok_or(Error::SessionNotActive)?; - // Get stack trace to find current position - let frames = sess.stack_trace(1).await?; - let frame = frames.first().ok_or_else(|| { - Error::Internal("No stack frame available".to_string()) - })?; + // Preserve the frame selected by `frame`, `up`, or `down`. + let frame_index = sess.get_current_frame_index(); + let frame = sess.select_frame(frame_index).await?; // Read source file let source_path = frame @@ -409,94 +390,48 @@ async fn handle_command_inner( } // === Async === - Command::Await { timeout_secs } => { - let sess = session.as_mut().ok_or(Error::SessionNotActive)?; - - // Process any pending events - sess.process_events().await?; - - // If we're already stopped, fetch stack trace and return stop info - if sess.state() == SessionState::Stopped { - // Fetch stack trace to get source location info - let (source, line, column) = match sess.stack_trace(1).await { - Ok(ref frames) => extract_source_location(frames), - Err(_) => (None, None, None), - }; - - let result = StopResult { - reason: sess.stopped_reason().unwrap_or("unknown").to_string(), - description: None, - thread_id: sess.stopped_thread(), - all_threads_stopped: true, - hit_breakpoint_ids: vec![], - source, - line, - column, - }; - return Ok(serde_json::to_value(result)?); - } - - if sess.state() == SessionState::Exited { - return Ok(json!({ - "reason": "exited", - "exit_code": sess.exit_code().unwrap_or(0) - })); - } - - // Wait for stop event - let event = sess.wait_stopped(timeout_secs).await?; - - match event { - Event::Stopped(body) => { - // Fetch stack trace to get source location info - let (source, line, column) = match sess.stack_trace(1).await { - Ok(ref frames) => extract_source_location(frames), - Err(_) => (None, None, None), - }; - - let result = StopResult { - reason: body.reason, - description: body.description, - thread_id: body.thread_id, - all_threads_stopped: body.all_threads_stopped, - hit_breakpoint_ids: body.hit_breakpoint_ids, - source, - line, - column, - }; - Ok(serde_json::to_value(result)?) - } - Event::Exited(body) => Ok(json!({ - "reason": "exited", - "exit_code": body.exit_code - })), - Event::Terminated(_) => Ok(json!({ - "reason": "terminated" - })), - _ => Ok(json!({ - "reason": "unknown" - })), - } + Command::Await { .. } => { + // Await is handled by the connection task in the server, which + // waits on state snapshots so it never occupies the session actor. + // Reaching this arm means a bug in command routing. + Err(Error::Internal( + "await must be handled by the daemon connection layer".to_string(), + )) } // === Output === - Command::GetOutput { tail, clear } => { + Command::GetOutput { tail, clear, cursor } => { let sess = session.as_mut().ok_or(Error::SessionNotActive)?; - let events = sess.get_output(tail, clear); - - let output: String = events.iter().map(|e| e.output.as_str()).collect(); + // Make output visible immediately instead of waiting for the daemon's + // periodic event-processing tick. + sess.process_events().await?; + // `--tail` is documented in lines, while the DAP emits arbitrary + // output chunks. Read the full bounded buffer first, then trim the + // concatenated stream by lines so chunk boundaries are invisible. + let (events, next_cursor) = sess.read_output(cursor, clear); + + let all_output: String = events.iter().map(|e| e.output.as_str()).collect(); + let output = tail + .map(|line_count| tail_output_lines(&all_output, line_count)) + .unwrap_or(all_output); + let event_details: Vec<_> = events + .iter() + .map(|event| { + json!({ + "category": event.category, + "output": event.output, + }) + }) + .collect(); Ok(json!({ "output": output, - "count": events.len() + "count": events.len(), + "events": event_details, + "next_cursor": next_cursor, })) } - Command::SubscribeOutput => { - // TODO: Implement output streaming - Err(Error::Internal("Output streaming not yet implemented".to_string())) - } - // === Shutdown === Command::Shutdown => { // Signal daemon to exit @@ -529,7 +464,12 @@ fn read_source_context(path: &str, current_line: u32, context: usize) -> Result< })?; let lines: Vec<&str> = content.lines().collect(); - let current_idx = (current_line as usize).saturating_sub(1); + if lines.is_empty() { + return Ok(Vec::new()); + } + let current_idx = (current_line as usize) + .saturating_sub(1) + .min(lines.len() - 1); let start = current_idx.saturating_sub(context); let end = (current_idx + context + 1).min(lines.len()); @@ -546,3 +486,41 @@ fn read_source_context(path: &str, current_line: u32, context: usize) -> Result< Ok(result) } + +/// Return the last `line_count` lines while preserving a trailing newline. +fn tail_output_lines(output: &str, line_count: usize) -> String { + if line_count == 0 || output.is_empty() { + return String::new(); + } + + let lines: Vec<_> = output.lines().collect(); + let start = lines.len().saturating_sub(line_count); + let mut result = lines[start..].join("\n"); + if output.ends_with('\n') && !result.is_empty() { + result.push('\n'); + } + result +} + +#[cfg(test)] +mod tests { + use super::tail_output_lines; + + #[test] + fn tail_output_is_line_based_across_dap_chunks() { + assert_eq!(tail_output_lines("first\nsecond\nthird\n", 2), "second\nthird\n"); + assert_eq!(tail_output_lines("first\nsecond\n", 0), ""); + assert_eq!(tail_output_lines("only", 3), "only"); + } + + #[test] + fn source_context_handles_adapter_lines_beyond_the_file() { + let directory = tempfile::tempdir().unwrap(); + let source = directory.path().join("short.c"); + std::fs::write(&source, "one\ntwo\n").unwrap(); + + let context = super::read_source_context(source.to_str().unwrap(), 99, 1).unwrap(); + assert_eq!(context.len(), 2); + assert!(!context.iter().any(|line| line.is_current)); + } +} diff --git a/src/daemon/mod.rs b/src/daemon/mod.rs index d48f34f..d56f34b 100644 --- a/src/daemon/mod.rs +++ b/src/daemon/mod.rs @@ -3,6 +3,7 @@ //! The daemon is spawned automatically by CLI commands and maintains //! persistent debug sessions across CLI invocations. +mod actor; mod handler; mod server; mod session; diff --git a/src/daemon/server.rs b/src/daemon/server.rs index 14e3f3c..9390a9e 100644 --- a/src/daemon/server.rs +++ b/src/daemon/server.rs @@ -1,42 +1,48 @@ -//! Daemon server - IPC listener and main event loop +//! Daemon server - IPC listener and connection tasks +//! +//! The accept loop spawns one task per client connection, so clients are +//! handled concurrently. All session access goes through the session actor +//! (see `actor.rs`); `await` is handled here by waiting on state snapshots so +//! it never blocks other clients. +use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; use interprocess::local_socket::traits::tokio::Listener as ListenerTrait; +use serde_json::json; use tokio::io::BufReader; +use tokio::sync::{mpsc, oneshot, watch}; -use crate::common::{config::Config, paths, Result}; +use crate::common::{config::Config, error::IpcError, paths, Error, Result}; use crate::ipc::{ - protocol::{Command, Request, Response}, + protocol::{Command, Request, Response, StackFrameInfo, StopResult}, transport, }; -use super::handler; -use super::session::DebugSession; +use super::actor::{self, ActorRequest, SessionSnapshot}; +use super::session::SessionState; + +/// Handles shared by every connection task. +#[derive(Clone)] +struct Shared { + requests: mpsc::Sender, + snapshots: watch::Receiver, + shutdown_tx: Arc>, + shutdown_rx: watch::Receiver, + last_activity: Arc>, +} /// Main daemon server pub struct Daemon { /// Configuration - config: Config, - /// Active debug session - session: Option, - /// Last activity timestamp for idle timeout - last_activity: Instant, - /// Whether shutdown was requested - shutdown_requested: bool, + config: Arc, } impl Daemon { /// Create a new daemon instance pub async fn new() -> Result { - let config = Config::load()?; - - Ok(Self { - config, - session: None, - last_activity: Instant::now(), - shutdown_requested: false, - }) + let config = Arc::new(Config::load()?); + Ok(Self { config }) } /// Run the daemon main loop @@ -47,30 +53,51 @@ impl Daemon { let idle_timeout = Duration::from_secs(self.config.daemon.idle_timeout_minutes * 60); + let (request_tx, request_rx) = mpsc::channel(32); + let (snapshot_tx, snapshot_rx) = watch::channel(SessionSnapshot::default()); + let (shutdown_tx, shutdown_rx) = watch::channel(false); + + let actor_task = tokio::spawn(actor::run(self.config.clone(), request_rx, snapshot_tx)); + + let shared = Shared { + requests: request_tx, + snapshots: snapshot_rx, + shutdown_tx: Arc::new(shutdown_tx), + shutdown_rx, + last_activity: Arc::new(Mutex::new(Instant::now())), + }; + let mut shutdown_rx = shared.shutdown_rx.clone(); + loop { // Check for idle timeout - if self.session.is_none() && self.last_activity.elapsed() > idle_timeout { + let idle = !shared.snapshots.borrow().session_active + && shared.last_activity.lock().unwrap().elapsed() > idle_timeout; + if idle { tracing::info!("Idle timeout reached, shutting down daemon"); break; } - // Check for shutdown request - if self.shutdown_requested { + if *shutdown_rx.borrow() { tracing::info!("Shutdown requested, exiting"); break; } // Accept connections with timeout, also handle signals - if self.run_select_loop(&listener).await? { + if run_select_loop(&listener, &shared, &mut shutdown_rx).await? { break; } } - // Cleanup + // Cleanup: tell connection tasks to exit, then let the actor stop the + // session once every request sender is dropped. tracing::info!("Cleaning up daemon resources"); - if let Some(mut session) = self.session.take() { - tracing::debug!("Stopping debug session"); - let _ = session.stop().await; + let _ = shared.shutdown_tx.send(true); + drop(shared); + if tokio::time::timeout(Duration::from_secs(10), actor_task) + .await + .is_err() + { + tracing::warn!("Session actor did not shut down in time"); } // Remove socket file @@ -79,170 +106,312 @@ impl Daemon { Ok(()) } +} - /// Run one iteration of the select loop, returns true if should break - #[cfg(unix)] - async fn run_select_loop( - &mut self, - listener: &transport::platform::Listener, - ) -> Result { - use tokio::signal::unix::{signal, SignalKind}; - - // Set up signal handlers (recreated each iteration to avoid lifetime issues) - let mut sigterm = signal(SignalKind::terminate()) - .expect("Failed to create SIGTERM handler"); - let mut sigint = signal(SignalKind::interrupt()) - .expect("Failed to create SIGINT handler"); - - tokio::select! { - // Handle SIGTERM (graceful shutdown) - _ = sigterm.recv() => { - tracing::info!("Received SIGTERM, shutting down gracefully"); - Ok(true) - } - // Handle SIGINT (Ctrl+C) - _ = sigint.recv() => { - tracing::info!("Received SIGINT (Ctrl+C), shutting down gracefully"); - Ok(true) - } - accept_result = listener.accept() => { - match accept_result { - Ok(stream) => { - self.last_activity = Instant::now(); - if let Err(e) = self.handle_client(stream).await { - tracing::error!("Error handling client: {}", e); - } - } - Err(e) => { - tracing::error!("Accept error: {}", e); - } +/// Run one iteration of the select loop, returns true if should break +#[cfg(unix)] +async fn run_select_loop( + listener: &transport::platform::Listener, + shared: &Shared, + shutdown_rx: &mut watch::Receiver, +) -> Result { + use tokio::signal::unix::{signal, SignalKind}; + + // Set up signal handlers (recreated each iteration to avoid lifetime issues) + let mut sigterm = signal(SignalKind::terminate()).expect("Failed to create SIGTERM handler"); + let mut sigint = signal(SignalKind::interrupt()).expect("Failed to create SIGINT handler"); + + tokio::select! { + // Handle SIGTERM (graceful shutdown) + _ = sigterm.recv() => { + tracing::info!("Received SIGTERM, shutting down gracefully"); + Ok(true) + } + // Handle SIGINT (Ctrl+C) + _ = sigint.recv() => { + tracing::info!("Received SIGINT (Ctrl+C), shutting down gracefully"); + Ok(true) + } + _ = shutdown_rx.changed() => Ok(*shutdown_rx.borrow()), + accept_result = listener.accept() => { + match accept_result { + Ok(stream) => { + *shared.last_activity.lock().unwrap() = Instant::now(); + tokio::spawn(handle_client(stream, shared.clone())); } - Ok(false) - } - _ = tokio::time::sleep(Duration::from_secs(1)) => { - // Periodic wakeup to check idle timeout - // Also process any pending events - if let Some(session) = &mut self.session { - if let Err(e) = session.process_events().await { - tracing::warn!("Error processing events: {}", e); - } + Err(e) => { + tracing::error!("Accept error: {}", e); } - Ok(false) } + Ok(false) + } + _ = tokio::time::sleep(Duration::from_secs(1)) => { + // Periodic wakeup to check idle timeout + Ok(false) } } +} - /// Run one iteration of the select loop (Windows version) - #[cfg(not(unix))] - async fn run_select_loop( - &mut self, - listener: &transport::platform::Listener, - ) -> Result { - tokio::select! { - accept_result = listener.accept() => { - match accept_result { - Ok(stream) => { - self.last_activity = Instant::now(); - if let Err(e) = self.handle_client(stream).await { - tracing::error!("Error handling client: {}", e); - } - } - Err(e) => { - tracing::error!("Accept error: {}", e); - } +/// Run one iteration of the select loop (Windows version) +#[cfg(not(unix))] +async fn run_select_loop( + listener: &transport::platform::Listener, + shared: &Shared, + shutdown_rx: &mut watch::Receiver, +) -> Result { + tokio::select! { + _ = shutdown_rx.changed() => Ok(*shutdown_rx.borrow()), + accept_result = listener.accept() => { + match accept_result { + Ok(stream) => { + *shared.last_activity.lock().unwrap() = Instant::now(); + tokio::spawn(handle_client(stream, shared.clone())); } - Ok(false) - } - _ = tokio::time::sleep(Duration::from_secs(1)) => { - // Periodic wakeup to check idle timeout - if let Some(session) = &mut self.session { - if let Err(e) = session.process_events().await { - tracing::warn!("Error processing events: {}", e); - } + Err(e) => { + tracing::error!("Accept error: {}", e); } - Ok(false) } + Ok(false) + } + _ = tokio::time::sleep(Duration::from_secs(1)) => { + // Periodic wakeup to check idle timeout + Ok(false) } } +} - /// Handle a single client connection - async fn handle_client( - &mut self, - stream: transport::platform::Stream, - ) -> Result<()> { - let (reader, mut writer) = tokio::io::split(stream); - let mut reader = BufReader::new(reader); +/// Handle a single client connection +async fn handle_client(stream: transport::platform::Stream, mut shared: Shared) { + let (reader, mut writer) = tokio::io::split(stream); + let mut reader = BufReader::new(reader); - // Read and process commands until client disconnects - loop { - // Read request with timeout - let request_data = tokio::select! { - result = transport::recv_message(&mut reader) => { - match result { - Ok(data) => data, - Err(e) if e.kind() == std::io::ErrorKind::UnexpectedEof => { - // Client disconnected - tracing::debug!("Client disconnected"); - break; - } - Err(e) => { - tracing::error!("Error reading request: {}", e); - break; - } + // Read and process commands until client disconnects + loop { + // Read request with timeout + let request_data = tokio::select! { + result = transport::recv_message(&mut reader) => { + match result { + Ok(data) => data, + Err(e) if e.kind() == std::io::ErrorKind::UnexpectedEof => { + // Client disconnected + tracing::debug!("Client disconnected"); + break; + } + Err(e) => { + tracing::error!("Error reading request: {}", e); + break; } } - _ = tokio::time::sleep(Duration::from_secs(300)) => { - // Client timeout - tracing::debug!("Client timeout"); + } + _ = shared.shutdown_rx.changed() => { + tracing::debug!("Daemon shutting down, closing client connection"); + break; + } + _ = tokio::time::sleep(Duration::from_secs(300)) => { + // Client timeout + tracing::debug!("Client timeout"); + break; + } + }; + + // Parse request + let request: Request = match serde_json::from_slice(&request_data) { + Ok(req) => req, + Err(e) => { + tracing::error!("Invalid request: {}", e); + let response = Response::error( + 0, + IpcError { + code: "INVALID_REQUEST".to_string(), + message: e.to_string(), + }, + ); + if send_response(&mut writer, &response).await.is_err() { break; } - }; + continue; + } + }; - // Parse request - let request: Request = match serde_json::from_slice(&request_data) { - Ok(req) => req, - Err(e) => { - tracing::error!("Invalid request: {}", e); - let response = Response::error( - 0, - crate::common::error::IpcError { - code: "INVALID_REQUEST".to_string(), - message: e.to_string(), - }, - ); - let json = serde_json::to_vec(&response)?; - transport::send_message(&mut writer, &json).await?; - continue; + tracing::debug!("Received command: {:?}", request.command); + *shared.last_activity.lock().unwrap() = Instant::now(); + + let mut shutdown_after_reply = false; + let response = match request.command { + Command::Shutdown => { + shutdown_after_reply = true; + Response::ok(request.id) + } + // Await waits on state snapshots so a stopped/exited transition can + // be observed without occupying the session actor; other clients + // stay free to send pause/continue while this connection waits. + Command::Await { timeout_secs } => { + match await_stop(timeout_secs, &shared).await { + Ok(result) => Response::success(request.id, result), + Err(e) => Response::error(request.id, IpcError::from(&e)), } - }; + } + command => dispatch(request.id, command, &shared).await, + }; - tracing::debug!("Received command: {:?}", request.command); + if send_response(&mut writer, &response).await.is_err() { + break; + } + *shared.last_activity.lock().unwrap() = Instant::now(); - // Check for shutdown command - if matches!(request.command, Command::Shutdown) { - self.shutdown_requested = true; - let response = Response::ok(request.id); - let json = serde_json::to_vec(&response)?; - transport::send_message(&mut writer, &json).await?; - break; - } + if shutdown_after_reply { + let _ = shared.shutdown_tx.send(true); + break; + } + } +} + +async fn send_response( + writer: &mut (impl tokio::io::AsyncWrite + Unpin), + response: &Response, +) -> std::io::Result<()> { + let json = serde_json::to_vec(response).map_err(std::io::Error::other)?; + transport::send_message(writer, &json).await +} + +/// Forward a command to the session actor and wait for its reply. +async fn dispatch(id: u64, command: Command, shared: &Shared) -> Response { + let (reply_tx, reply_rx) = oneshot::channel(); + let request = ActorRequest { + id, + command, + reply: reply_tx, + }; + + if shared.requests.send(request).await.is_err() { + return daemon_stopping_response(id); + } + + match reply_rx.await { + Ok(response) => response, + Err(_) => daemon_stopping_response(id), + } +} + +fn daemon_stopping_response(id: u64) -> Response { + Response::error( + id, + IpcError::from(&Error::Internal("daemon is shutting down".to_string())), + ) +} - // Handle command - let response = handler::handle_command( - &mut self.session, - &self.config, - request.id, - request.command, - ) - .await; +/// Wait for the session to stop by watching state snapshots. +async fn await_stop(timeout_secs: u64, shared: &Shared) -> Result { + let mut snapshots = shared.snapshots.clone(); + let deadline = tokio::time::Instant::now() + Duration::from_secs(timeout_secs); - // Send response - let json = serde_json::to_vec(&response)?; - transport::send_message(&mut writer, &json).await?; + loop { + let snapshot = snapshots.borrow_and_update().clone(); - self.last_activity = Instant::now(); + if !snapshot.session_active { + return Err(Error::SessionNotActive); } - Ok(()) + match snapshot.state { + Some(SessionState::Stopped) => { + return build_stop_result(&snapshot, shared).await; + } + Some(SessionState::Exited) => { + // Adapters that report an exit code send Exited; a bare + // Terminated event leaves the code unknown. + return Ok(match snapshot.exit_code { + Some(code) => json!({ "reason": "exited", "exit_code": code }), + None => json!({ "reason": "terminated" }), + }); + } + _ => {} + } + + let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); + if remaining.is_zero() { + return Err(Error::AwaitTimeout(timeout_secs)); + } + + match tokio::time::timeout(remaining, snapshots.changed()).await { + Ok(Ok(())) => {} + Ok(Err(_)) => { + return Err(Error::Internal("daemon is shutting down".to_string())); + } + Err(_) => return Err(Error::AwaitTimeout(timeout_secs)), + } } } + +/// Build the stop result for `await`, including the top frame's location. +async fn build_stop_result( + snapshot: &SessionSnapshot, + shared: &Shared, +) -> Result { + let (source, line, column) = fetch_stop_location(shared).await; + + let result = match &snapshot.last_stop { + Some(body) => StopResult { + reason: body.reason.clone(), + description: body.description.clone(), + thread_id: body.thread_id, + all_threads_stopped: body.all_threads_stopped, + hit_breakpoint_ids: body.hit_breakpoint_ids.clone(), + source, + line, + column, + }, + // Stopped without an adapter event (attach, stop-on-entry). + None => StopResult { + reason: snapshot + .stopped_reason + .clone() + .unwrap_or_else(|| "unknown".to_string()), + description: None, + thread_id: snapshot.stopped_thread, + all_threads_stopped: true, + hit_breakpoint_ids: vec![], + source, + line, + column, + }, + }; + + Ok(serde_json::to_value(result)?) +} + +/// Ask the actor for the top stack frame and extract filename/line/column. +async fn fetch_stop_location(shared: &Shared) -> (Option, Option, Option) { + let response = dispatch( + 0, + Command::StackTrace { + thread_id: None, + limit: 1, + }, + shared, + ) + .await; + + let frames: Vec = match response + .result + .and_then(|mut r| r.get_mut("frames").map(serde_json::Value::take)) + .map(serde_json::from_value) + { + Some(Ok(frames)) if response.success => frames, + _ => return (None, None, None), + }; + + let Some(frame) = frames.first() else { + return (None, None, None); + }; + + // Report just the filename, matching the pre-actor await output. + let source = frame.source.as_ref().map(|path| { + std::path::Path::new(path) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or(path) + .to_string() + }); + + (source, frame.line, frame.column) +} diff --git a/src/daemon/session.rs b/src/daemon/session/mod.rs similarity index 51% rename from src/daemon/session.rs rename to src/daemon/session/mod.rs index ecbe99f..f01974f 100644 --- a/src/daemon/session.rs +++ b/src/daemon/session/mod.rs @@ -1,73 +1,30 @@ -//! Debug session state machine +//! Debug session façade //! -//! Manages the lifecycle of a debug session from initialization through -//! termination. +//! `DebugSession` orchestrates the DAP adapter (`DapClient` transport), feeds +//! adapter events into the reducer-owned `SessionModel`, and routes debuggee +//! output into the cursor-based `OutputBuffer`. It manages the lifecycle of a +//! debug session from initialization through termination. -use std::collections::{HashMap, VecDeque}; +mod model; +mod output; + +pub use model::SessionState; +pub use output::OutputEvent; + +use std::collections::HashMap; use std::path::{Path, PathBuf}; use tokio::sync::mpsc; -use crate::common::{config::{Config, TransportMode}, Error, Result}; +use crate::common::{config::{adapter_fallback_names, Config, TransportMode}, Error, Result}; use crate::dap::{ self, Breakpoint, Capabilities, DapClient, Event, FunctionBreakpoint, LaunchArguments, - AttachArguments, Scope, SourceBreakpoint, StackFrame, Thread, Variable, + AttachArguments, Scope, SourceBreakpoint, StackFrame, StoppedEventBody, Thread, Variable, }; use crate::ipc::protocol::{BreakpointInfo, BreakpointLocation}; -/// Debug session state -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum SessionState { - /// No active session - Idle, - /// DAP adapter starting - Initializing, - /// Setting initial breakpoints - Configuring, - /// Program is running - Running, - /// Program has stopped (breakpoint, step, exception) - Stopped, - /// Program has exited - Exited, - /// Session is terminating - Terminating, -} - -impl std::fmt::Display for SessionState { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Idle => write!(f, "idle"), - Self::Initializing => write!(f, "initializing"), - Self::Configuring => write!(f, "configuring"), - Self::Running => write!(f, "running"), - Self::Stopped => write!(f, "stopped"), - Self::Exited => write!(f, "exited"), - Self::Terminating => write!(f, "terminating"), - } - } -} - -/// Stored breakpoint information -#[derive(Debug, Clone)] -struct StoredBreakpoint { - id: u32, - location: BreakpointLocation, - condition: Option, - hit_count: Option, - enabled: bool, - verified: bool, - actual_line: Option, - message: Option, -} - -/// Output event for buffering -#[derive(Debug, Clone)] -pub struct OutputEvent { - pub category: String, - pub output: String, - pub timestamp: std::time::Instant, -} +use model::{SessionModel, StoredBreakpoint}; +use output::OutputBuffer; /// Debug session managing a DAP connection pub struct DebugSession { @@ -75,52 +32,18 @@ pub struct DebugSession { client: DapClient, /// Event receiver from DAP client events_rx: mpsc::UnboundedReceiver, - /// Current session state - state: SessionState, /// Adapter capabilities capabilities: Capabilities, /// Program being debugged program: PathBuf, - /// Program arguments - args: Vec, /// Adapter name adapter_name: String, /// Whether we launched (vs attached) launched: bool, - /// All breakpoints by source file - source_breakpoints: HashMap>, - /// Function breakpoints - function_breakpoints: Vec, - /// Next breakpoint ID - next_bp_id: u32, - /// Cached threads - threads: Vec, - /// Currently selected thread (may differ from stopped thread) - selected_thread: Option, - /// Currently stopped thread - stopped_thread: Option, - /// Reason for last stop - stopped_reason: Option, - /// Hit breakpoint IDs from last stop - hit_breakpoints: Vec, - /// Current frame index (0 = top of stack) - current_frame_index: usize, - /// Current frame ID (for variable inspection) - current_frame: Option, - /// Cached stack frames for current stop - cached_frames: Vec, - /// Output buffer - output_buffer: VecDeque, - /// Maximum output buffer size - max_output_events: usize, - /// Maximum output buffer bytes - max_output_bytes: usize, - /// Current output buffer byte count - current_output_bytes: usize, - /// Exit code if program exited - exit_code: Option, - /// DAP request timeout - dap_request_timeout: std::time::Duration, + /// Reducer-owned session state + model: SessionModel, + /// Bounded output buffer + output_buffer: OutputBuffer, } impl DebugSession { @@ -137,7 +60,8 @@ impl DebugSession { let adapter_name = adapter_name.unwrap_or_else(|| config.defaults.adapter.clone()); let adapter_config = config.get_adapter(&adapter_name).ok_or_else(|| { - Error::adapter_not_found(&adapter_name, &[&adapter_name]) + let searched = adapter_fallback_names(&adapter_name); + Error::adapter_not_found(&adapter_name, &searched) })?; tracing::info!( @@ -164,6 +88,7 @@ impl DebugSession { // Initialize the adapter with timeout let init_timeout = std::time::Duration::from_secs(config.timeouts.dap_initialize_secs); let request_timeout = std::time::Duration::from_secs(config.timeouts.dap_request_secs); + client.set_request_timeout(request_timeout); // Initialize the adapter with timeout tracing::debug!(timeout_secs = init_timeout.as_secs(), "Sending DAP initialize request"); @@ -223,18 +148,13 @@ impl DebugSession { stop_on_entry, "Sending DAP launch request" ); - - // For Python/debugpy, we need to use non-blocking launch because debugpy - // doesn't respond to launch until after configurationDone is sent. - // We send launch, wait for initialized, send configurationDone, then - // the launch response arrives. - if is_python { - client.launch_no_wait(launch_args).await?; - tracing::debug!("DAP launch request sent (no-wait mode for Python)"); - } else { - client.launch(launch_args).await?; - tracing::debug!("DAP launch request successful"); - } + + // The DAP protocol permits adapters to defer the launch response until + // after configurationDone. GDB and debugpy both do so, while other + // adapters may respond immediately. Waiting here deadlocks the former + // before we can send their initial breakpoints and configurationDone. + client.launch_no_wait(launch_args).await?; + tracing::debug!("DAP launch request sent (deferred-response mode)"); // Wait for initialized event (comes after launch per DAP spec) tracing::debug!(timeout_secs = request_timeout.as_secs(), "Waiting for DAP initialized event"); @@ -243,71 +163,93 @@ impl DebugSession { // Set initial breakpoints before configurationDone // This is required for adapters that don't support stopOnEntry (e.g., cdt-gdb-adapter) - let has_initial_breakpoints = !initial_breakpoints.is_empty(); - if has_initial_breakpoints { + let mut source_breakpoints = HashMap::new(); + let mut function_breakpoints = Vec::new(); + let mut next_bp_id = 1; + + if !initial_breakpoints.is_empty() { tracing::debug!(count = initial_breakpoints.len(), "Setting initial breakpoints"); // Group breakpoints by type (source vs function) - let mut source_bps: std::collections::HashMap> = std::collections::HashMap::new(); + let mut source_bps: HashMap> = HashMap::new(); let mut function_bps: Vec = Vec::new(); for bp_str in &initial_breakpoints { - match BreakpointLocation::parse(bp_str) { - Ok(BreakpointLocation::Line { file, line }) => { - source_bps.entry(file).or_default().push(dap::SourceBreakpoint { - line, - column: None, - condition: None, - hit_condition: None, - log_message: None, - }); + let location = BreakpointLocation::parse(bp_str)?; + let bp_id = next_bp_id; + next_bp_id += 1; + + match &location { + BreakpointLocation::Line { file, line } => { + source_bps + .entry(file.clone()) + .or_default() + .push(dap::SourceBreakpoint { + line: *line, + column: None, + condition: None, + hit_condition: None, + log_message: None, + }); + source_breakpoints + .entry(file.clone()) + .or_insert_with(Vec::new) + .push(StoredBreakpoint { + id: bp_id, + location, + condition: None, + hit_count: None, + enabled: true, + verified: false, + actual_line: None, + message: None, + }); } - Ok(BreakpointLocation::Function { name }) => { + BreakpointLocation::Function { name } => { function_bps.push(dap::FunctionBreakpoint { - name, + name: name.clone(), condition: None, hit_condition: None, }); - } - Err(e) => { - tracing::warn!(breakpoint = %bp_str, error = %e, "Failed to parse initial breakpoint"); + function_breakpoints.push(StoredBreakpoint { + id: bp_id, + location, + condition: None, + hit_count: None, + enabled: true, + verified: false, + actual_line: None, + message: None, + }); } } } // Set source breakpoints for (file, bps) in source_bps { - match client.set_breakpoints(&file, bps).await { - Ok(results) => { - for bp in results { - tracing::debug!( - verified = bp.verified, - line = bp.line, - "Initial source breakpoint set" - ); - } - } - Err(e) => { - tracing::warn!(file = %file.display(), error = %e, "Failed to set initial breakpoints"); + let results = client.set_breakpoints(&file, bps).await?; + if let Some(stored_bps) = source_breakpoints.get_mut(&file) { + for (stored, result) in stored_bps.iter_mut().zip(results.iter()) { + stored.verified = result.verified; + stored.actual_line = result.line; + stored.message = result.message.clone(); } } } // Set function breakpoints if !function_bps.is_empty() { - match client.set_function_breakpoints(function_bps).await { - Ok(results) => { - for bp in results { - tracing::debug!( - verified = bp.verified, - line = bp.line, - "Initial function breakpoint set" - ); - } - } - Err(e) => { - tracing::warn!(error = %e, "Failed to set initial function breakpoints"); - } + if !capabilities.supports_function_breakpoints { + return Err(Error::Internal( + "Debug adapter does not support function breakpoints. Use file:line format instead." + .to_string(), + )); + } + let results = client.set_function_breakpoints(function_bps).await?; + for (stored, result) in function_breakpoints.iter_mut().zip(results.iter()) { + stored.verified = result.verified; + stored.actual_line = result.line; + stored.message = result.message.clone(); } } } @@ -330,32 +272,23 @@ impl DebugSession { SessionState::Running }; + let mut model = SessionModel::new(initial_state); + model.source_breakpoints = source_breakpoints; + model.function_breakpoints = function_breakpoints; + model.next_bp_id = next_bp_id; + Ok(Self { client, events_rx, - state: initial_state, capabilities, program: program.to_path_buf(), - args, adapter_name, launched: true, - source_breakpoints: HashMap::new(), - function_breakpoints: Vec::new(), - next_bp_id: 1, - threads: Vec::new(), - selected_thread: None, - stopped_thread: None, - stopped_reason: None, - hit_breakpoints: Vec::new(), - current_frame_index: 0, - current_frame: None, - cached_frames: Vec::new(), - output_buffer: VecDeque::new(), - max_output_events: config.output.max_events, - max_output_bytes: config.output.max_bytes_mb * 1024 * 1024, - current_output_bytes: 0, - exit_code: None, - dap_request_timeout: request_timeout, + model, + output_buffer: OutputBuffer::new( + config.output.max_events, + config.output.max_bytes_mb * 1024 * 1024, + ), }) } @@ -368,7 +301,8 @@ impl DebugSession { let adapter_name = adapter_name.unwrap_or_else(|| config.defaults.adapter.clone()); let adapter_config = config.get_adapter(&adapter_name).ok_or_else(|| { - Error::adapter_not_found(&adapter_name, &[&adapter_name]) + let searched = adapter_fallback_names(&adapter_name); + Error::adapter_not_found(&adapter_name, &searched) })?; tracing::info!( @@ -390,6 +324,7 @@ impl DebugSession { // Get configured timeouts let init_timeout = std::time::Duration::from_secs(config.timeouts.dap_initialize_secs); let request_timeout = std::time::Duration::from_secs(config.timeouts.dap_request_secs); + client.set_request_timeout(request_timeout); let capabilities = client.initialize_with_timeout(&adapter_name, init_timeout).await?; @@ -412,38 +347,33 @@ impl DebugSession { .take_event_receiver() .ok_or_else(|| Error::Internal("Failed to get event receiver".to_string()))?; + // Attached processes start stopped + let mut model = SessionModel::new(SessionState::Stopped); + model.stopped_reason = Some("attach".to_string()); + Ok(Self { client, events_rx, - state: SessionState::Stopped, // Attached processes start stopped capabilities, program: PathBuf::from(format!("pid:{}", pid)), - args: Vec::new(), adapter_name, launched: false, - source_breakpoints: HashMap::new(), - function_breakpoints: Vec::new(), - next_bp_id: 1, - threads: Vec::new(), - selected_thread: None, - stopped_thread: None, - stopped_reason: Some("attach".to_string()), - hit_breakpoints: Vec::new(), - current_frame_index: 0, - current_frame: None, - cached_frames: Vec::new(), - output_buffer: VecDeque::new(), - max_output_events: config.output.max_events, - max_output_bytes: config.output.max_bytes_mb * 1024 * 1024, - current_output_bytes: 0, - exit_code: None, - dap_request_timeout: request_timeout, + model, + output_buffer: OutputBuffer::new( + config.output.max_events, + config.output.max_bytes_mb * 1024 * 1024, + ), }) } /// Get current state pub fn state(&self) -> SessionState { - self.state + self.model.state + } + + /// Full body of the last stopped event, if the program is stopped + pub fn last_stop(&self) -> Option<&StoppedEventBody> { + self.model.last_stop.as_ref() } /// Get program path @@ -458,17 +388,17 @@ impl DebugSession { /// Get stopped thread ID pub fn stopped_thread(&self) -> Option { - self.stopped_thread + self.model.stopped_thread } - /// Get stopped reason + /// Get stop reason pub fn stopped_reason(&self) -> Option<&str> { - self.stopped_reason.as_deref() + self.model.stopped_reason.as_deref() } - /// Get exit code if exited + /// Get exit code pub fn exit_code(&self) -> Option { - self.exit_code + self.model.exit_code } /// Process pending events @@ -476,7 +406,7 @@ impl DebugSession { let mut events = Vec::new(); while let Ok(event) = self.events_rx.try_recv() { - self.handle_event(&event); + self.dispatch_event(&event); events.push(event); } @@ -487,176 +417,19 @@ impl DebugSession { /// This ensures we don't lose state updates from events while clearing the queue fn drain_pending_events(&mut self) { while let Ok(event) = self.events_rx.try_recv() { - self.handle_event(&event); + self.dispatch_event(&event); } } - /// Handle a single event - fn handle_event(&mut self, event: &Event) { + /// Route a single event: output goes to the buffer, everything else is + /// state and reduces into the model. + fn dispatch_event(&mut self, event: &Event) { match event { - Event::Stopped(body) => { - self.state = SessionState::Stopped; - self.stopped_thread = body.thread_id; - self.stopped_reason = Some(body.reason.clone()); - self.hit_breakpoints = body.hit_breakpoint_ids.clone(); - // Reset frame tracking on stop - user starts at top of stack - self.current_frame = None; - self.current_frame_index = 0; - self.cached_frames.clear(); - tracing::debug!("Stopped: {:?}", body); - } - Event::Continued { thread_id, .. } => { - self.state = SessionState::Running; - self.stopped_thread = None; - self.stopped_reason = None; - self.hit_breakpoints.clear(); - self.current_frame = None; - self.current_frame_index = 0; - self.cached_frames.clear(); - tracing::debug!("Continued: thread {}", thread_id); - } - Event::Exited(body) => { - self.state = SessionState::Exited; - self.exit_code = Some(body.exit_code); - tracing::info!("Program exited with code {}", body.exit_code); - } - Event::Terminated(_) => { - self.state = SessionState::Exited; - tracing::info!("Session terminated"); - } Event::Output(body) => { - let category = body.category.clone().unwrap_or_else(|| "console".to_string()); - self.buffer_output(&category, &body.output); - } - Event::Thread(body) => { - tracing::debug!("Thread {}: {}", body.thread_id, body.reason); - // Update thread list if needed - if body.reason == "exited" { - self.threads.retain(|t| t.id != body.thread_id); - // Clear selected thread if it was the one that exited - if self.selected_thread == Some(body.thread_id) { - self.selected_thread = None; - } - } - } - Event::Breakpoint { reason, breakpoint } => { - tracing::debug!("Breakpoint {}: {:?}", reason, breakpoint); - // Update breakpoint status if we get change notifications - if let Some(bp_id) = breakpoint.id { - self.update_breakpoint_from_event(bp_id as u32, breakpoint); - } - } - _ => {} - } - } - - /// Update breakpoint status from a breakpoint event - fn update_breakpoint_from_event(&mut self, _id: u32, bp: &dap::Breakpoint) { - // Try to match by line/source to update verification status - if let (Some(source), Some(line)) = (&bp.source, bp.line) { - if let Some(path) = &source.path { - let path = PathBuf::from(path); - if let Some(stored_bps) = self.source_breakpoints.get_mut(&path) { - for stored in stored_bps.iter_mut() { - if let BreakpointLocation::Line { line: stored_line, .. } = &stored.location { - if *stored_line == line || stored.actual_line == Some(line) { - stored.verified = bp.verified; - stored.actual_line = bp.line; - stored.message = bp.message.clone(); - break; - } - } - } - } - } - } - } - - /// Buffer output for later retrieval - /// - /// Enforces both max_output_events and max_output_bytes limits. - /// If a single output message exceeds max_output_bytes, it is truncated. - fn buffer_output(&mut self, category: &str, output: &str) { - // Truncate oversized messages to prevent exceeding limits - let output = if output.len() > self.max_output_bytes { - tracing::warn!( - "Output message ({} bytes) exceeds max buffer size ({} bytes), truncating", - output.len(), - self.max_output_bytes - ); - // Truncate to fit, trying to break at a char boundary - let truncated: String = output.chars().take(self.max_output_bytes).collect(); - truncated - } else { - output.to_string() - }; - - let output_bytes = output.len(); - - // Enforce byte limit - remove oldest entries until we have space - while self.current_output_bytes + output_bytes > self.max_output_bytes - && !self.output_buffer.is_empty() - { - if let Some(removed) = self.output_buffer.pop_front() { - self.current_output_bytes = self.current_output_bytes.saturating_sub(removed.output.len()); - } - } - - // Enforce event count limit - while self.output_buffer.len() >= self.max_output_events && !self.output_buffer.is_empty() { - if let Some(removed) = self.output_buffer.pop_front() { - self.current_output_bytes = self.current_output_bytes.saturating_sub(removed.output.len()); - } - } - - // Add the new output - self.output_buffer.push_back(OutputEvent { - category: category.to_string(), - output, - timestamp: std::time::Instant::now(), - }); - self.current_output_bytes += output_bytes; - } - - /// Wait for the program to stop - /// - /// This method waits for a stop event (Stopped, Exited, or Terminated) to arrive - /// through the event channel. Since the background reader task in DapClient - /// continuously reads events from the adapter, we just need to wait on the channel. - pub async fn wait_stopped(&mut self, timeout_secs: u64) -> Result { - let timeout = std::time::Duration::from_secs(timeout_secs); - let deadline = tokio::time::Instant::now() + timeout; - - loop { - // Calculate remaining time - let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); - if remaining.is_zero() { - return Err(Error::AwaitTimeout(timeout_secs)); - } - - // Wait for next event with timeout - match tokio::time::timeout(remaining, self.events_rx.recv()).await { - Ok(Some(event)) => { - self.handle_event(&event); - - match &event { - Event::Stopped(_) | Event::Exited(_) | Event::Terminated(_) => { - return Ok(event); - } - _ => { - // Continue waiting for stop event - } - } - } - Ok(None) => { - // Channel closed - adapter crashed or terminated - return Err(Error::AdapterCrashed); - } - Err(_) => { - // Timeout elapsed - return Err(Error::AwaitTimeout(timeout_secs)); - } + let category = body.category.as_deref().unwrap_or("console"); + self.output_buffer.push(category, &body.output); } + _ => self.model.apply(event), } } @@ -667,67 +440,73 @@ impl DebugSession { condition: Option, hit_count: Option, ) -> Result { - let bp_id = self.next_bp_id; - self.next_bp_id += 1; + let bp_id = self.model.next_bp_id; + self.model.next_bp_id += 1; + + let stored = StoredBreakpoint { + id: bp_id, + location: location.clone(), + condition: condition.clone(), + hit_count, + enabled: true, + verified: false, + actual_line: None, + message: None, + }; match &location { BreakpointLocation::Line { file, line: _ } => { // Add to our tracking - let stored = StoredBreakpoint { - id: bp_id, - location: location.clone(), - condition: condition.clone(), - hit_count, - enabled: true, - verified: false, - actual_line: None, - message: None, - }; - - let entry = self.source_breakpoints.entry(file.clone()).or_default(); - entry.push(stored); + self.model + .source_breakpoints + .entry(file.clone()) + .or_default() + .push(stored); // Send to adapter let source_bps = self.collect_source_breakpoints(file); - let results = self.client.set_breakpoints(file, source_bps).await?; + let results = match self.client.set_breakpoints(file, source_bps).await { + Ok(results) => results, + Err(error) => { + if let Some(breakpoints) = self.model.source_breakpoints.get_mut(file) { + breakpoints.retain(|breakpoint| breakpoint.id != bp_id); + } + return Err(error); + } + }; // Update verification status self.update_source_breakpoint_status(file, &results); - - // Find our breakpoint in results - let info = self.get_breakpoint_info(bp_id)?; - Ok(info) } BreakpointLocation::Function { name: _ } => { - let stored = StoredBreakpoint { - id: bp_id, - location: location.clone(), - condition: condition.clone(), - hit_count, - enabled: true, - verified: false, - actual_line: None, - message: None, - }; - - self.function_breakpoints.push(stored); + self.model.function_breakpoints.push(stored); // Send all function breakpoints let func_bps = self.collect_function_breakpoints(); - let results = self.client.set_function_breakpoints(func_bps).await?; + let results = match self.client.set_function_breakpoints(func_bps).await { + Ok(results) => results, + Err(error) => { + self.model + .function_breakpoints + .retain(|breakpoint| breakpoint.id != bp_id); + return Err(error); + } + }; // Update verification status self.update_function_breakpoint_status(&results); - - let info = self.get_breakpoint_info(bp_id)?; - Ok(info) } } + + self.model + .breakpoint_info(bp_id) + .ok_or(Error::BreakpointNotFound { id: bp_id }) } /// Collect source breakpoints for a file fn collect_source_breakpoints(&self, file: &Path) -> Vec { - self.source_breakpoints + self.model + .source_breakpoints .get(file) .map(|bps| { bps.iter() @@ -752,7 +531,8 @@ impl DebugSession { /// Collect function breakpoints fn collect_function_breakpoints(&self) -> Vec { - self.function_breakpoints + self.model + .function_breakpoints .iter() .filter(|bp| bp.enabled) .map(|bp| { @@ -771,7 +551,7 @@ impl DebugSession { /// Update source breakpoint status from adapter response fn update_source_breakpoint_status(&mut self, file: &Path, results: &[Breakpoint]) { - if let Some(stored) = self.source_breakpoints.get_mut(file) { + if let Some(stored) = self.model.source_breakpoints.get_mut(file) { // Match by line number (best effort) for (stored_bp, result) in stored.iter_mut().zip(results.iter()) { stored_bp.verified = result.verified; @@ -783,77 +563,43 @@ impl DebugSession { /// Update function breakpoint status from adapter response fn update_function_breakpoint_status(&mut self, results: &[Breakpoint]) { - for (stored_bp, result) in self.function_breakpoints.iter_mut().zip(results.iter()) { + for (stored_bp, result) in self.model.function_breakpoints.iter_mut().zip(results.iter()) { stored_bp.verified = result.verified; stored_bp.actual_line = result.line; stored_bp.message = result.message.clone(); } } - /// Get breakpoint info by ID - fn get_breakpoint_info(&self, id: u32) -> Result { - // Search source breakpoints - for (file, bps) in &self.source_breakpoints { - if let Some(bp) = bps.iter().find(|bp| bp.id == id) { - return Ok(BreakpointInfo { - id: bp.id, - verified: bp.verified, - source: Some(file.to_string_lossy().into_owned()), - line: bp.actual_line.or(match &bp.location { - BreakpointLocation::Line { line, .. } => Some(*line), - _ => None, - }), - message: bp.message.clone(), - enabled: bp.enabled, - condition: bp.condition.clone(), - hit_count: bp.hit_count, - }); - } - } - - // Search function breakpoints - if let Some(bp) = self.function_breakpoints.iter().find(|bp| bp.id == id) { - return Ok(BreakpointInfo { - id: bp.id, - verified: bp.verified, - source: match &bp.location { - BreakpointLocation::Function { name } => Some(name.clone()), - _ => None, - }, - line: bp.actual_line, - message: bp.message.clone(), - enabled: bp.enabled, - condition: bp.condition.clone(), - hit_count: bp.hit_count, - }); - } - - Err(Error::BreakpointNotFound { id }) - } - /// Remove a breakpoint by ID pub async fn remove_breakpoint(&mut self, id: u32) -> Result<()> { // Find and remove from source breakpoints - let mut file_to_update = None; - for (file, bps) in &mut self.source_breakpoints { + let mut source_breakpoint = None; + for (file, bps) in &mut self.model.source_breakpoints { if let Some(pos) = bps.iter().position(|bp| bp.id == id) { - bps.remove(pos); - file_to_update = Some(file.clone()); + source_breakpoint = Some((file.clone(), pos, bps.remove(pos))); break; } } - if let Some(file) = file_to_update { + if let Some((file, position, removed)) = source_breakpoint { let source_bps = self.collect_source_breakpoints(&file); - self.client.set_breakpoints(&file, source_bps).await?; + if let Err(error) = self.client.set_breakpoints(&file, source_bps).await { + if let Some(breakpoints) = self.model.source_breakpoints.get_mut(&file) { + breakpoints.insert(position, removed); + } + return Err(error); + } return Ok(()); } // Try function breakpoints - if let Some(pos) = self.function_breakpoints.iter().position(|bp| bp.id == id) { - self.function_breakpoints.remove(pos); + if let Some(pos) = self.model.function_breakpoints.iter().position(|bp| bp.id == id) { + let removed = self.model.function_breakpoints.remove(pos); let func_bps = self.collect_function_breakpoints(); - self.client.set_function_breakpoints(func_bps).await?; + if let Err(error) = self.client.set_function_breakpoints(func_bps).await { + self.model.function_breakpoints.insert(pos, removed); + return Err(error); + } return Ok(()); } @@ -863,58 +609,22 @@ impl DebugSession { /// Remove all breakpoints pub async fn remove_all_breakpoints(&mut self) -> Result<()> { // Clear source breakpoints - let files: Vec<_> = self.source_breakpoints.keys().cloned().collect(); + let files: Vec<_> = self.model.source_breakpoints.keys().cloned().collect(); for file in files { self.client.set_breakpoints(&file, vec![]).await?; + self.model.source_breakpoints.remove(&file); } - self.source_breakpoints.clear(); // Clear function breakpoints self.client.set_function_breakpoints(vec![]).await?; - self.function_breakpoints.clear(); + self.model.function_breakpoints.clear(); Ok(()) } /// List all breakpoints pub fn list_breakpoints(&self) -> Vec { - let mut result = Vec::new(); - - for (file, bps) in &self.source_breakpoints { - for bp in bps { - result.push(BreakpointInfo { - id: bp.id, - verified: bp.verified, - source: Some(file.to_string_lossy().into_owned()), - line: bp.actual_line.or(match &bp.location { - BreakpointLocation::Line { line, .. } => Some(*line), - _ => None, - }), - message: bp.message.clone(), - enabled: bp.enabled, - condition: bp.condition.clone(), - hit_count: bp.hit_count, - }); - } - } - - for bp in &self.function_breakpoints { - result.push(BreakpointInfo { - id: bp.id, - verified: bp.verified, - source: match &bp.location { - BreakpointLocation::Function { name } => Some(name.clone()), - _ => None, - }, - line: bp.actual_line, - message: bp.message.clone(), - enabled: bp.enabled, - condition: bp.condition.clone(), - hit_count: bp.hit_count, - }); - } - - result + self.model.list_breakpoints() } /// Continue execution @@ -927,9 +637,7 @@ impl DebugSession { let thread_id = self.get_thread_id().await?; self.client.continue_execution(thread_id).await?; - self.state = SessionState::Running; - self.stopped_thread = None; - self.stopped_reason = None; + self.model.on_resumed(); Ok(()) } @@ -943,7 +651,7 @@ impl DebugSession { let thread_id = self.get_thread_id().await?; self.client.next(thread_id).await?; - self.state = SessionState::Running; + self.model.on_resumed(); Ok(()) } @@ -957,7 +665,7 @@ impl DebugSession { let thread_id = self.get_thread_id().await?; self.client.step_in(thread_id).await?; - self.state = SessionState::Running; + self.model.on_resumed(); Ok(()) } @@ -971,15 +679,15 @@ impl DebugSession { let thread_id = self.get_thread_id().await?; self.client.step_out(thread_id).await?; - self.state = SessionState::Running; + self.model.on_resumed(); Ok(()) } /// Pause execution pub async fn pause(&mut self) -> Result<()> { - if self.state != SessionState::Running { - return Err(Error::invalid_state("pause", &self.state.to_string())); + if self.model.state != SessionState::Running { + return Err(Error::invalid_state("pause", &self.model.state.to_string())); } let thread_id = self.get_thread_id().await?; @@ -989,24 +697,24 @@ impl DebugSession { } /// Get stack trace - pub async fn stack_trace(&mut self, limit: usize) -> Result> { + pub async fn stack_trace( + &mut self, + requested_thread: Option, + limit: usize, + ) -> Result> { self.ensure_stopped()?; - let thread_id = self.get_thread_id().await?; - let frames = self.client.stack_trace(thread_id, limit as i64).await?; - - // Cache the top frame ID - if let Some(frame) = frames.first() { - self.current_frame = Some(frame.id); - } - - Ok(frames) + let thread_id = match requested_thread { + Some(thread_id) => thread_id, + None => self.get_thread_id().await?, + }; + self.client.stack_trace(thread_id, limit as i64).await } /// Get threads pub async fn get_threads(&mut self) -> Result> { - self.threads = self.client.threads().await?; - Ok(self.threads.clone()) + self.model.threads = self.client.threads().await?; + Ok(self.model.threads.clone()) } /// Get scopes for current frame @@ -1014,7 +722,7 @@ impl DebugSession { self.ensure_stopped()?; // Auto-fetch top frame if no frame specified and current_frame is not set - let frame_id = match frame_id.or(self.current_frame) { + let frame_id = match frame_id.or(self.model.current_frame) { Some(id) => id, None => { // Fetch stack trace to get the top frame @@ -1023,7 +731,7 @@ impl DebugSession { let frame = frames.first().ok_or_else(|| { Error::Internal("No stack frames available".to_string()) })?; - self.current_frame = Some(frame.id); + self.model.current_frame = Some(frame.id); frame.id } }; @@ -1064,14 +772,14 @@ impl DebugSession { self.ensure_stopped()?; // Auto-fetch top frame if no frame specified and current_frame is not set - let frame_id = match frame_id.or(self.current_frame) { + let frame_id = match frame_id.or(self.model.current_frame) { Some(id) => Some(id), None => { // Fetch stack trace to get the top frame let thread_id = self.get_thread_id().await?; let frames = self.client.stack_trace(thread_id, 1).await?; if let Some(frame) = frames.first() { - self.current_frame = Some(frame.id); + self.model.current_frame = Some(frame.id); Some(frame.id) } else { None @@ -1081,31 +789,27 @@ impl DebugSession { self.client.evaluate(expression, frame_id, context).await } - /// Get buffered output - pub fn get_output(&mut self, tail: Option, clear: bool) -> Vec { - let result: Vec = if let Some(n) = tail { - self.output_buffer.iter().rev().take(n).cloned().rev().collect() - } else { - self.output_buffer.iter().cloned().collect() - }; - + /// Read buffered output from `cursor` (or the oldest retained event) and + /// return the events plus the cursor for the next read. Clearing discards + /// the buffer for every client but keeps cursors monotonic. + pub fn read_output(&mut self, cursor: Option, clear: bool) -> (Vec, u64) { + let result = self.output_buffer.read_from(cursor); if clear { self.output_buffer.clear(); } - result } /// Detach from the debuggee (keep it running) pub async fn detach(&mut self) -> Result<()> { - self.state = SessionState::Terminating; + self.model.state = SessionState::Terminating; self.client.disconnect(false).await?; Ok(()) } /// Stop the debuggee and terminate session pub async fn stop(&mut self) -> Result<()> { - self.state = SessionState::Terminating; + self.model.state = SessionState::Terminating; self.client.disconnect(self.launched).await?; self.client.terminate().await?; Ok(()) @@ -1118,41 +822,33 @@ impl DebugSession { /// user should be instructed to use 'debugger stop' then 'debugger start'. pub async fn restart(&mut self) -> Result<()> { self.client.restart(false).await?; - self.state = SessionState::Running; // Clear frame/stop state since we're restarting - self.stopped_thread = None; - self.stopped_reason = None; - self.current_frame = None; - self.current_frame_index = 0; - self.cached_frames.clear(); + self.model.on_resumed(); Ok(()) } /// Select a thread for debugging operations /// - /// Returns an error if the thread is not found in the current thread list. - /// Note: The thread list may be stale; call `get_threads()` first to refresh. - pub fn select_thread(&mut self, thread_id: i64) -> Result<()> { - // Verify thread exists in our known thread list - if !self.threads.iter().any(|t| t.id == thread_id) { + /// Returns an error if the thread is not currently reported by the adapter. + pub async fn select_thread(&mut self, thread_id: i64) -> Result<()> { + self.model.threads = self.client.threads().await?; + if !self.model.threads.iter().any(|t| t.id == thread_id) { return Err(Error::Internal(format!( "Thread {} not found. Use 'threads' command to see available threads.", thread_id ))); } - self.selected_thread = Some(thread_id); + self.model.selected_thread = Some(thread_id); // Reset frame selection when switching threads - self.current_frame_index = 0; - self.current_frame = None; - self.cached_frames.clear(); + self.model.reset_frame_selection(); Ok(()) } /// Get the currently selected thread (for UI display) pub fn get_selected_thread(&self) -> Option { - self.selected_thread.or(self.stopped_thread) + self.model.selected_thread.or(self.model.stopped_thread) } /// Select a stack frame by index (0 = top/innermost) @@ -1160,41 +856,41 @@ impl DebugSession { self.ensure_stopped()?; // Fetch frames if not cached or if requesting beyond cache - if self.cached_frames.is_empty() || frame_index >= self.cached_frames.len() { + if self.model.cached_frames.is_empty() || frame_index >= self.model.cached_frames.len() { let thread_id = self.get_thread_id().await?; // Fetch enough frames to include the requested one let needed = (frame_index + 1).max(20); - self.cached_frames = self.client.stack_trace(thread_id, needed as i64).await?; + self.model.cached_frames = self.client.stack_trace(thread_id, needed as i64).await?; } - if frame_index >= self.cached_frames.len() { + if frame_index >= self.model.cached_frames.len() { return Err(Error::FrameNotFound(frame_index)); } - self.current_frame_index = frame_index; - self.current_frame = Some(self.cached_frames[frame_index].id); + self.model.current_frame_index = frame_index; + self.model.current_frame = Some(self.model.cached_frames[frame_index].id); - Ok(self.cached_frames[frame_index].clone()) + Ok(self.model.cached_frames[frame_index].clone()) } /// Move up the stack (to caller frame) pub async fn frame_up(&mut self) -> Result { - let new_index = self.current_frame_index + 1; + let new_index = self.model.current_frame_index + 1; self.select_frame(new_index).await } /// Move down the stack (toward innermost/current frame) pub async fn frame_down(&mut self) -> Result { - if self.current_frame_index == 0 { + if self.model.current_frame_index == 0 { return Err(Error::invalid_state("frame down", "already at innermost frame")); } - let new_index = self.current_frame_index - 1; + let new_index = self.model.current_frame_index - 1; self.select_frame(new_index).await } /// Get current frame index pub fn get_current_frame_index(&self) -> usize { - self.current_frame_index + self.model.current_frame_index } /// Enable a breakpoint @@ -1210,34 +906,56 @@ impl DebugSession { /// Set breakpoint enabled state async fn set_breakpoint_enabled(&mut self, id: u32, enabled: bool) -> Result<()> { // Find and update the breakpoint - let mut file_to_update = None; - let mut is_function_bp = false; + let mut source_breakpoint = None; - for (file, bps) in &mut self.source_breakpoints { + for (file, bps) in &mut self.model.source_breakpoints { if let Some(bp) = bps.iter_mut().find(|bp| bp.id == id) { + let previous_enabled = bp.enabled; bp.enabled = enabled; - file_to_update = Some(file.clone()); + source_breakpoint = Some((file.clone(), previous_enabled)); break; } } - if file_to_update.is_none() { - if let Some(bp) = self.function_breakpoints.iter_mut().find(|bp| bp.id == id) { + let mut function_previous_enabled = None; + if source_breakpoint.is_none() { + if let Some(bp) = self.model.function_breakpoints.iter_mut().find(|bp| bp.id == id) { + function_previous_enabled = Some(bp.enabled); bp.enabled = enabled; - is_function_bp = true; } else { return Err(Error::BreakpointNotFound { id }); } } // Re-send breakpoints to adapter - if let Some(file) = file_to_update { + if let Some((file, previous_enabled)) = source_breakpoint { let source_bps = self.collect_source_breakpoints(&file); - let results = self.client.set_breakpoints(&file, source_bps).await?; + let results = match self.client.set_breakpoints(&file, source_bps).await { + Ok(results) => results, + Err(error) => { + if let Some(bp) = self + .model + .source_breakpoints + .get_mut(&file) + .and_then(|breakpoints| breakpoints.iter_mut().find(|bp| bp.id == id)) + { + bp.enabled = previous_enabled; + } + return Err(error); + } + }; self.update_source_breakpoint_status(&file, &results); - } else if is_function_bp { + } else if let Some(previous_enabled) = function_previous_enabled { let func_bps = self.collect_function_breakpoints(); - let results = self.client.set_function_breakpoints(func_bps).await?; + let results = match self.client.set_function_breakpoints(func_bps).await { + Ok(results) => results, + Err(error) => { + if let Some(bp) = self.model.function_breakpoints.iter_mut().find(|bp| bp.id == id) { + bp.enabled = previous_enabled; + } + return Err(error); + } + }; self.update_function_breakpoint_status(&results); } @@ -1266,33 +984,34 @@ impl DebugSession { /// Ensure we're in stopped state for inspection commands fn ensure_stopped(&self) -> Result<()> { - match self.state { + match self.model.state { SessionState::Stopped => Ok(()), SessionState::Exited => { - Err(Error::ProgramExited(self.exit_code.unwrap_or(0))) + Err(Error::ProgramExited(self.model.exit_code.unwrap_or(0))) } - _ => Err(Error::invalid_state("inspect", &self.state.to_string())), + _ => Err(Error::invalid_state("inspect", &self.model.state.to_string())), } } /// Get a thread ID (preferring selected > stopped > first) async fn get_thread_id(&mut self) -> Result { // Prefer explicitly selected thread - if let Some(id) = self.selected_thread { + if let Some(id) = self.model.selected_thread { return Ok(id); } // Fall back to stopped thread - if let Some(id) = self.stopped_thread { + if let Some(id) = self.model.stopped_thread { return Ok(id); } // Fetch threads and use the first one - if self.threads.is_empty() { - self.threads = self.client.threads().await?; + if self.model.threads.is_empty() { + self.model.threads = self.client.threads().await?; } - self.threads + self.model + .threads .first() .map(|t| t.id) .ok_or_else(|| Error::Internal("No threads available".to_string())) diff --git a/src/daemon/session/model.rs b/src/daemon/session/model.rs new file mode 100644 index 0000000..50a6c4e --- /dev/null +++ b/src/daemon/session/model.rs @@ -0,0 +1,251 @@ +//! Reducer-owned session state. +//! +//! `SessionModel` holds everything derived from DAP events and command +//! side-effects: lifecycle state, thread/frame selection, stop details, +//! breakpoint bookkeeping, and exit status. It contains no I/O; `DebugSession` +//! orchestrates the adapter and feeds events into `apply`. + +use std::collections::HashMap; +use std::path::PathBuf; + +use crate::dap::{self, Event, StackFrame, StoppedEventBody, Thread}; +use crate::ipc::protocol::{BreakpointInfo, BreakpointLocation}; + +/// Debug session state +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SessionState { + /// Program is running + Running, + /// Program has stopped (breakpoint, step, exception) + Stopped, + /// Program has exited + Exited, + /// Session is terminating + Terminating, +} + +impl std::fmt::Display for SessionState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Running => write!(f, "running"), + Self::Stopped => write!(f, "stopped"), + Self::Exited => write!(f, "exited"), + Self::Terminating => write!(f, "terminating"), + } + } +} + +/// Stored breakpoint information +#[derive(Debug, Clone)] +pub(super) struct StoredBreakpoint { + pub(super) id: u32, + pub(super) location: BreakpointLocation, + pub(super) condition: Option, + pub(super) hit_count: Option, + pub(super) enabled: bool, + pub(super) verified: bool, + pub(super) actual_line: Option, + pub(super) message: Option, +} + +impl StoredBreakpoint { + pub(super) fn info(&self, source: Option) -> BreakpointInfo { + BreakpointInfo { + id: self.id, + verified: self.verified, + source, + line: self.actual_line.or(match &self.location { + BreakpointLocation::Line { line, .. } => Some(*line), + _ => None, + }), + message: self.message.clone(), + enabled: self.enabled, + condition: self.condition.clone(), + hit_count: self.hit_count, + } + } +} + +/// State container reduced from DAP events and command side-effects. +#[derive(Debug)] +pub(super) struct SessionModel { + /// Current session state + pub(super) state: SessionState, + /// All breakpoints by source file + pub(super) source_breakpoints: HashMap>, + /// Function breakpoints + pub(super) function_breakpoints: Vec, + /// Next breakpoint ID + pub(super) next_bp_id: u32, + /// Cached threads + pub(super) threads: Vec, + /// Currently selected thread (may differ from stopped thread) + pub(super) selected_thread: Option, + /// Currently stopped thread + pub(super) stopped_thread: Option, + /// Reason for last stop + pub(super) stopped_reason: Option, + /// Full body of the last stopped event, cleared when execution resumes + pub(super) last_stop: Option, + /// Hit breakpoint IDs from last stop + pub(super) hit_breakpoints: Vec, + /// Current frame index (0 = top of stack) + pub(super) current_frame_index: usize, + /// Current frame ID (for variable inspection) + pub(super) current_frame: Option, + /// Cached stack frames for current stop + pub(super) cached_frames: Vec, + /// Exit code if program exited + pub(super) exit_code: Option, +} + +impl SessionModel { + pub(super) fn new(initial_state: SessionState) -> Self { + Self { + state: initial_state, + source_breakpoints: HashMap::new(), + function_breakpoints: Vec::new(), + next_bp_id: 1, + threads: Vec::new(), + selected_thread: None, + stopped_thread: None, + stopped_reason: None, + last_stop: None, + hit_breakpoints: Vec::new(), + current_frame_index: 0, + current_frame: None, + cached_frames: Vec::new(), + exit_code: None, + } + } + + /// Reduce a single DAP event into the model. Output events are not state + /// and are routed to the output buffer by `DebugSession` instead. + pub(super) fn apply(&mut self, event: &Event) { + match event { + Event::Stopped(body) => { + self.state = SessionState::Stopped; + self.stopped_thread = body.thread_id; + self.selected_thread = body.thread_id; + self.stopped_reason = Some(body.reason.clone()); + self.last_stop = Some(body.clone()); + self.hit_breakpoints = body.hit_breakpoint_ids.clone(); + // Reset frame tracking on stop - user starts at top of stack + self.reset_frame_selection(); + tracing::debug!("Stopped: {:?}", body); + } + Event::Continued { thread_id, .. } => { + self.on_resumed(); + tracing::debug!("Continued: thread {}", thread_id); + } + Event::Exited(body) => { + self.state = SessionState::Exited; + self.selected_thread = None; + self.exit_code = Some(body.exit_code); + tracing::info!("Program exited with code {}", body.exit_code); + } + Event::Terminated(_) => { + self.state = SessionState::Exited; + self.selected_thread = None; + tracing::info!("Session terminated"); + } + Event::Thread(body) => { + tracing::debug!("Thread {}: {}", body.thread_id, body.reason); + // Update thread list if needed + if body.reason == "exited" { + self.threads.retain(|t| t.id != body.thread_id); + // Clear selected thread if it was the one that exited + if self.selected_thread == Some(body.thread_id) { + self.selected_thread = None; + } + } + } + Event::Breakpoint { reason, breakpoint } => { + tracing::debug!("Breakpoint {}: {:?}", reason, breakpoint); + // Update breakpoint status if we get change notifications + if let Some(bp_id) = breakpoint.id { + self.update_breakpoint_from_event(bp_id, breakpoint); + } + } + _ => {} + } + } + + /// Clear stop-derived state when execution resumes (continue/step). + pub(super) fn on_resumed(&mut self) { + self.state = SessionState::Running; + self.selected_thread = None; + self.stopped_thread = None; + self.stopped_reason = None; + self.last_stop = None; + self.hit_breakpoints.clear(); + self.reset_frame_selection(); + } + + pub(super) fn reset_frame_selection(&mut self) { + self.current_frame = None; + self.current_frame_index = 0; + self.cached_frames.clear(); + } + + /// Update breakpoint status from a breakpoint event + fn update_breakpoint_from_event(&mut self, _id: u32, bp: &dap::Breakpoint) { + // Try to match by line/source to update verification status + if let (Some(source), Some(line)) = (&bp.source, bp.line) { + if let Some(path) = &source.path { + let path = PathBuf::from(path); + if let Some(stored_bps) = self.source_breakpoints.get_mut(&path) { + for stored in stored_bps.iter_mut() { + if let BreakpointLocation::Line { line: stored_line, .. } = &stored.location { + if *stored_line == line || stored.actual_line == Some(line) { + stored.verified = bp.verified; + stored.actual_line = bp.line; + stored.message = bp.message.clone(); + break; + } + } + } + } + } + } + } + + /// Get breakpoint info by ID + pub(super) fn breakpoint_info(&self, id: u32) -> Option { + for (file, bps) in &self.source_breakpoints { + if let Some(bp) = bps.iter().find(|bp| bp.id == id) { + return Some(bp.info(Some(file.to_string_lossy().into_owned()))); + } + } + + self.function_breakpoints + .iter() + .find(|bp| bp.id == id) + .map(|bp| { + bp.info(match &bp.location { + BreakpointLocation::Function { name } => Some(name.clone()), + _ => None, + }) + }) + } + + /// List all breakpoints + pub(super) fn list_breakpoints(&self) -> Vec { + let mut result = Vec::new(); + + for (file, bps) in &self.source_breakpoints { + for bp in bps { + result.push(bp.info(Some(file.to_string_lossy().into_owned()))); + } + } + + for bp in &self.function_breakpoints { + result.push(bp.info(match &bp.location { + BreakpointLocation::Function { name } => Some(name.clone()), + _ => None, + })); + } + + result + } +} diff --git a/src/daemon/session/output.rs b/src/daemon/session/output.rs new file mode 100644 index 0000000..ead90be --- /dev/null +++ b/src/daemon/session/output.rs @@ -0,0 +1,214 @@ +//! Bounded, cursor-based buffer for debuggee output. +//! +//! Every buffered event has a monotonic sequence number, so independent +//! clients can each read from their own cursor without consuming data the +//! others have not seen. Old events expire only through the event-count and +//! byte-count bounds (or an explicit clear); sequence numbers keep advancing +//! across trimming and clears, so a stale cursor simply resumes at the oldest +//! retained event. + +use std::collections::VecDeque; + +/// Output event for buffering +#[derive(Debug, Clone)] +pub struct OutputEvent { + pub category: String, + pub output: String, +} + +/// Bounded, in-memory buffer for debuggee output. +/// +/// The DAP reader can produce arbitrarily large output events, so the buffer +/// enforces both an event-count and a byte-count limit. Keeping this logic +/// separate from `DebugSession` makes its accounting independently testable. +#[derive(Debug)] +pub struct OutputBuffer { + events: VecDeque, + /// Sequence number of the front event; `first_seq + events.len()` is the + /// cursor position just past the newest event. + first_seq: u64, + max_events: usize, + max_bytes: usize, + current_bytes: usize, +} + +impl OutputBuffer { + pub fn new(max_events: usize, max_bytes: usize) -> Self { + Self { + events: VecDeque::new(), + first_seq: 0, + max_events, + max_bytes, + current_bytes: 0, + } + } + + pub fn push(&mut self, category: &str, output: &str) { + if self.max_events == 0 || self.max_bytes == 0 { + return; + } + + let output = truncate_utf8_to_bytes(output, self.max_bytes); + if output.is_empty() { + return; + } + let output_bytes = output.len(); + + while self.current_bytes + output_bytes > self.max_bytes && !self.events.is_empty() { + self.pop_oldest(); + } + + while self.events.len() >= self.max_events && !self.events.is_empty() { + self.pop_oldest(); + } + + self.events.push_back(OutputEvent { + category: category.to_string(), + output, + }); + self.current_bytes += output_bytes; + } + + fn pop_oldest(&mut self) { + if let Some(removed) = self.events.pop_front() { + self.current_bytes = self.current_bytes.saturating_sub(removed.output.len()); + self.first_seq += 1; + } + } + + /// Read events at or after `cursor` without consuming them. + /// + /// `None` reads from the oldest retained event. Returns the events and the + /// cursor to pass on the next read to see only newer output. Cursors older + /// than the retained window resume at the oldest event; cursors from the + /// future clamp to the end. + pub fn read_from(&self, cursor: Option) -> (Vec, u64) { + let end = self.first_seq + self.events.len() as u64; + let start = cursor + .unwrap_or(self.first_seq) + .clamp(self.first_seq, end); + + let events = self + .events + .iter() + .skip((start - self.first_seq) as usize) + .cloned() + .collect(); + (events, end) + } + + /// Discard all buffered events. Sequence numbers keep advancing, so + /// cursors held by other clients stay valid. + pub fn clear(&mut self) { + self.first_seq += self.events.len() as u64; + self.events.clear(); + self.current_bytes = 0; + } +} + +/// Return the longest valid UTF-8 prefix that fits within `max_bytes`. +fn truncate_utf8_to_bytes(value: &str, max_bytes: usize) -> String { + if value.len() <= max_bytes { + return value.to_string(); + } + + let mut end = 0; + for (start, character) in value.char_indices() { + let character_end = start + character.len_utf8(); + if character_end > max_bytes { + break; + } + end = character_end; + } + + value[..end].to_string() +} + +#[cfg(test)] +mod tests { + use super::OutputBuffer; + + #[test] + fn clearing_output_resets_byte_accounting() { + let mut buffer = OutputBuffer::new(4, 4); + buffer.push("stdout", "abcd"); + + let (drained, _) = buffer.read_from(None); + assert_eq!(drained.len(), 1); + assert_eq!(drained[0].output, "abcd"); + + buffer.clear(); + assert_eq!(buffer.current_bytes, 0); + + buffer.push("stdout", "xyz"); + assert_eq!(buffer.current_bytes, 3); + assert_eq!(buffer.read_from(None).0[0].output, "xyz"); + } + + #[test] + fn output_is_truncated_on_a_utf8_boundary() { + let mut buffer = OutputBuffer::new(4, 5); + buffer.push("stdout", "ééé"); + + let (output, _) = buffer.read_from(None); + assert_eq!(output.len(), 1); + assert_eq!(output[0].output, "éé"); + assert_eq!(buffer.current_bytes, 4); + } + + #[test] + fn zero_sized_buffers_discard_output() { + let mut buffer = OutputBuffer::new(0, 32); + buffer.push("stdout", "discard me"); + assert!(buffer.read_from(None).0.is_empty()); + } + + #[test] + fn cursors_read_independently_and_survive_clears() { + let mut buffer = OutputBuffer::new(16, 1024); + buffer.push("stdout", "one"); + buffer.push("stdout", "two"); + + // Two clients each read everything from their own cursor. + let (a_events, a_cursor) = buffer.read_from(None); + let (b_events, b_cursor) = buffer.read_from(None); + assert_eq!(a_events.len(), 2); + assert_eq!(b_events.len(), 2); + assert_eq!(a_cursor, 2); + assert_eq!(b_cursor, 2); + + // New output is visible to both, exactly once, from their cursors. + buffer.push("stdout", "three"); + let (a_events, a_cursor) = buffer.read_from(Some(a_cursor)); + assert_eq!(a_events.len(), 1); + assert_eq!(a_events[0].output, "three"); + + // A clear keeps sequence numbers advancing. + buffer.clear(); + buffer.push("stdout", "four"); + let (a_events, _) = buffer.read_from(Some(a_cursor)); + assert_eq!(a_events.len(), 1); + assert_eq!(a_events[0].output, "four"); + + // A stale cursor from before trimming resumes at the oldest event. + let (b_events, _) = buffer.read_from(Some(b_cursor)); + assert_eq!(b_events.len(), 1); + assert_eq!(b_events[0].output, "four"); + } + + #[test] + fn trimming_by_bounds_advances_the_start_of_the_window() { + let mut buffer = OutputBuffer::new(2, 1024); + buffer.push("stdout", "one"); + buffer.push("stdout", "two"); + buffer.push("stdout", "three"); // evicts "one" + + let (events, cursor) = buffer.read_from(Some(0)); + assert_eq!(events.len(), 2); + assert_eq!(events[0].output, "two"); + assert_eq!(cursor, 3); + + // A future cursor clamps to the end instead of panicking. + assert!(buffer.read_from(Some(99)).0.is_empty()); + } +} diff --git a/src/dap/client.rs b/src/dap/client.rs index 75449b1..83fc034 100644 --- a/src/dap/client.rs +++ b/src/dap/client.rs @@ -87,6 +87,8 @@ pub struct DapClient { seq: AtomicI64, /// Adapter capabilities (populated after initialize) pub capabilities: Capabilities, + /// Default timeout for DAP requests after initialization. + request_timeout: Duration, /// Pending response waiters pending: PendingResponses, /// Channel for events (to session) @@ -141,6 +143,7 @@ impl DapClient { writer: DapWriter::Stdio(BufWriter::new(stdin)), seq: AtomicI64::new(1), capabilities: Capabilities::default(), + request_timeout: Duration::from_secs(30), pending, event_tx, event_rx: Some(event_rx), @@ -262,7 +265,6 @@ impl DapClient { // Retry TCP connection with exponential backoff // Handles adapters that need time to start listening (e.g., js-debug) let stream = { - let mut last_error = None; let mut delay = Duration::from_millis(100); let max_delay = Duration::from_millis(1000); let timeout_duration = Duration::from_secs(10); @@ -272,12 +274,11 @@ impl DapClient { match TcpStream::connect(&addr).await { Ok(s) => break s, Err(e) => { - last_error = Some(e); if start.elapsed() >= timeout_duration { let _ = adapter.start_kill(); return Err(Error::AdapterStartFailed(format!( "Failed to connect to adapter at {} after {:?}: {}", - addr, timeout_duration, last_error.unwrap() + addr, timeout_duration, e ))); } tokio::time::sleep(delay).await; @@ -306,6 +307,7 @@ impl DapClient { writer: DapWriter::Tcp(BufWriter::new(write_half)), seq: AtomicI64::new(1), capabilities: Capabilities::default(), + request_timeout: Duration::from_secs(30), pending, event_tx, event_rx: Some(event_rx), @@ -483,13 +485,31 @@ impl DapClient { self.event_rx.take() } + /// Set the timeout used by normal DAP requests after initialization. + pub fn set_request_timeout(&mut self, timeout: Duration) { + self.request_timeout = timeout; + } + /// Get the next sequence number fn next_seq(&self) -> i64 { self.seq.fetch_add(1, Ordering::SeqCst) } - /// Send a request and return its sequence number - async fn send_request(&mut self, command: &str, arguments: Option) -> Result { + /// Send a request and register its pending-response entry, returning the + /// sequence number and the response receiver. + /// + /// This is the single send path for every DAP request. The pending entry + /// is registered BEFORE the request is written, so a fast adapter response + /// cannot race the registration. Callers that wait drive the returned + /// receiver; callers that defer (e.g. `launch` on adapters that respond + /// only after `configurationDone`) drop it, which keeps the entry alive in + /// the pending map so the eventual response is not a spurious "unknown + /// request" warning. + async fn send_request_raw( + &mut self, + command: &str, + arguments: Option, + ) -> Result<(i64, oneshot::Receiver>)> { let seq = self.next_seq(); // Build request with or without arguments field @@ -511,8 +531,26 @@ impl DapClient { let json = serde_json::to_string(&request)?; tracing::trace!("DAP >>> {}", json); - codec::write_message(&mut self.writer, &json).await?; + let (tx, rx) = oneshot::channel(); + { + let mut pending_guard = self.pending.lock().await; + pending_guard.insert(seq, tx); + } + if let Err(error) = codec::write_message(&mut self.writer, &json).await { + let mut pending_guard = self.pending.lock().await; + pending_guard.remove(&seq); + return Err(error); + } + + Ok((seq, rx)) + } + + /// Send a request without waiting for its response and return its sequence + /// number. The response stays registered so adapters that defer it do not + /// produce a spurious "unknown request" warning. + async fn send_request(&mut self, command: &str, arguments: Option) -> Result { + let (seq, _rx) = self.send_request_raw(command, arguments).await?; Ok(seq) } @@ -522,56 +560,18 @@ impl DapClient { command: &str, arguments: Option, ) -> Result { - self.request_with_timeout(command, arguments, Duration::from_secs(30)).await + self.request_with_timeout(command, arguments, self.request_timeout) + .await } /// Send a request and wait for the response with configurable timeout - /// - /// Note: We register the pending response handler BEFORE sending the request - /// to avoid a race condition where a fast adapter response arrives before - /// we've set up the handler. pub async fn request_with_timeout( &mut self, command: &str, arguments: Option, timeout: Duration, ) -> Result { - let seq = self.next_seq(); - - // Build request with or without arguments field - let request = if let Some(ref args) = arguments { - serde_json::json!({ - "seq": seq, - "type": "request", - "command": command, - "arguments": args - }) - } else { - serde_json::json!({ - "seq": seq, - "type": "request", - "command": command - }) - }; - - // IMPORTANT: Register the pending response handler BEFORE sending the request - // to avoid race condition where fast adapter responds before we're ready - let (tx, rx) = oneshot::channel(); - { - let mut pending_guard = self.pending.lock().await; - pending_guard.insert(seq, tx); - } - - // Now send the request - let json = serde_json::to_string(&request)?; - tracing::trace!("DAP >>> {}", json); - - if let Err(e) = codec::write_message(&mut self.writer, &json).await { - // Remove the pending handler if send failed - let mut pending_guard = self.pending.lock().await; - pending_guard.remove(&seq); - return Err(e); - } + let (seq, rx) = self.send_request_raw(command, arguments).await?; // Wait for response with timeout let response = tokio::time::timeout(timeout, rx) diff --git a/src/dap/types.rs b/src/dap/types.rs index dc922aa..5eae8b0 100644 --- a/src/dap/types.rs +++ b/src/dap/types.rs @@ -482,7 +482,7 @@ pub struct Variable { // === Event Bodies === /// Stopped event body -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct StoppedEventBody { pub reason: String, diff --git a/src/ipc/protocol.rs b/src/ipc/protocol.rs index 0fd1e26..c543de3 100644 --- a/src/ipc/protocol.rs +++ b/src/ipc/protocol.rs @@ -189,11 +189,12 @@ pub enum Command { GetOutput { tail: Option, clear: bool, + /// Read only events at or after this cursor (from a previous + /// response's `next_cursor`). `None` reads the whole buffer. + #[serde(default)] + cursor: Option, }, - /// Subscribe to output events (for --follow) - SubscribeOutput, - // === Shutdown === /// Shutdown the daemon Shutdown, @@ -272,6 +273,9 @@ pub struct StatusResult { pub state: Option, pub program: Option, pub adapter: Option, + /// Thread selected for inspection commands, if any. + #[serde(default)] + pub selected_thread: Option, pub stopped_thread: Option, pub stopped_reason: Option, } diff --git a/src/setup/adapters/codelldb.rs b/src/setup/adapters/codelldb.rs index 9097480..12160d7 100644 --- a/src/setup/adapters/codelldb.rs +++ b/src/setup/adapters/codelldb.rs @@ -111,25 +111,10 @@ fn get_asset_pattern() -> Vec { let platform = platform_str(); let arch = arch_str(); - // Map arch names to CodeLLDB naming convention - let codelldb_arch = match arch { - "x86_64" => "x86_64", - "aarch64" => "aarch64", - _ => arch, - }; - - // Map platform names - let codelldb_platform = match platform { - "darwin" => "darwin", - "linux" => "linux", - "windows" => "windows", - _ => platform, - }; - vec![ - format!("codelldb-{}-{}.vsix", codelldb_arch, codelldb_platform), + format!("codelldb-{}-{}.vsix", arch, platform), // Alternative naming patterns - format!("codelldb-{}-{}-*.vsix", codelldb_arch, codelldb_platform), + format!("codelldb-{}-{}-*.vsix", arch, platform), ] } @@ -191,12 +176,10 @@ async fn install_from_github(opts: &InstallOptions) -> Result { #[cfg(unix)] { let lib_path = adapter_dir.join("extension").join("adapter"); - for entry in std::fs::read_dir(&lib_path)? { - if let Ok(entry) = entry { - let path = entry.path(); - if path.extension().map(|e| e == "so" || e == "dylib").unwrap_or(false) { - make_executable(&path)?; - } + for entry in std::fs::read_dir(&lib_path)?.flatten() { + let path = entry.path(); + if path.extension().map(|e| e == "so" || e == "dylib").unwrap_or(false) { + make_executable(&path)?; } } @@ -207,18 +190,16 @@ async fn install_from_github(opts: &InstallOptions) -> Result { let dir = lldb_dir.join(subdir); if dir.exists() { if let Ok(entries) = std::fs::read_dir(&dir) { - for entry in entries { - if let Ok(entry) = entry { - let path = entry.path(); - if path.is_file() { - // Log warning but don't fail installation - if let Err(e) = make_executable(&path) { - eprintln!( - "Warning: could not make {} executable: {}", - path.display(), - e - ); - } + for entry in entries.flatten() { + let path = entry.path(); + if path.is_file() { + // Log warning but don't fail installation + if let Err(e) = make_executable(&path) { + eprintln!( + "Warning: could not make {} executable: {}", + path.display(), + e + ); } } } diff --git a/src/setup/adapters/debugpy.rs b/src/setup/adapters/debugpy.rs index 4f362ee..ecb87d1 100644 --- a/src/setup/adapters/debugpy.rs +++ b/src/setup/adapters/debugpy.rs @@ -10,7 +10,7 @@ use crate::setup::installer::{ use crate::setup::registry::{DebuggerInfo, Platform}; use crate::setup::verifier::{verify_dap_adapter, VerifyResult}; use async_trait::async_trait; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; static INFO: DebuggerInfo = DebuggerInfo { id: "python", @@ -148,7 +148,7 @@ async fn find_python() -> Result { } /// Get the path to Python in a venv -fn get_venv_python(venv_dir: &PathBuf) -> PathBuf { +fn get_venv_python(venv_dir: &Path) -> PathBuf { if cfg!(windows) { venv_dir.join("Scripts").join("python.exe") } else { @@ -157,7 +157,7 @@ fn get_venv_python(venv_dir: &PathBuf) -> PathBuf { } /// Get the path to pip in a venv -fn get_venv_pip(venv_dir: &PathBuf) -> PathBuf { +fn get_venv_pip(venv_dir: &Path) -> PathBuf { if cfg!(windows) { venv_dir.join("Scripts").join("pip.exe") } else { diff --git a/src/setup/adapters/delve.rs b/src/setup/adapters/delve.rs index 723450b..6b2dc65 100644 --- a/src/setup/adapters/delve.rs +++ b/src/setup/adapters/delve.rs @@ -222,7 +222,7 @@ async fn install_from_github(opts: &InstallOptions) -> Result { _ => arch, }; - let patterns = vec![ + let patterns = [ format!("delve_{}_{}.tar.gz", platform, delve_arch), format!("delve_*_{}_{}.tar.gz", platform, delve_arch), ]; diff --git a/src/setup/adapters/gdb_common.rs b/src/setup/adapters/gdb_common.rs index 1188cc5..b4522a2 100644 --- a/src/setup/adapters/gdb_common.rs +++ b/src/setup/adapters/gdb_common.rs @@ -17,7 +17,7 @@ pub fn parse_gdb_version(output: &str) -> Option { if *part == "gdb" { if let Some(version) = parts.get(i + 1) { // Verify it starts with a digit (version number) - if version.chars().next().map_or(false, |c| c.is_ascii_digit()) { + if version.chars().next().is_some_and(|c| c.is_ascii_digit()) { return Some(version.to_string()); } } @@ -31,7 +31,7 @@ pub fn parse_gdb_version(output: &str) -> Option { .next() .and_then(|line| { line.split_whitespace() - .find(|token| token.chars().next().map_or(false, |c| c.is_ascii_digit())) + .find(|token| token.chars().next().is_some_and(|c| c.is_ascii_digit())) }) .map(|s| s.to_string()) } @@ -41,7 +41,7 @@ pub fn parse_gdb_version(output: &str) -> Option { /// Returns false on parse failure to prevent launching incompatible GDB pub fn is_gdb_version_sufficient(version: &str) -> bool { let parts: Vec<&str> = version.split('.').collect(); - let Some(major_str) = parts.get(0) else { + let Some(major_str) = parts.first() else { return false; }; let Some(minor_str) = parts.get(1) else { @@ -60,7 +60,7 @@ pub fn is_gdb_version_sufficient(version: &str) -> bool { /// Retrieves GDB version by executing --version flag /// /// Returns None on exec failure or unparseable output -pub async fn get_gdb_version(path: &std::path::PathBuf) -> Option { +pub async fn get_gdb_version(path: &std::path::Path) -> Option { let output = tokio::process::Command::new(path) .arg("--version") .output() diff --git a/src/setup/adapters/js_debug.rs b/src/setup/adapters/js_debug.rs index ab6f60e..ee0f0e3 100644 --- a/src/setup/adapters/js_debug.rs +++ b/src/setup/adapters/js_debug.rs @@ -10,7 +10,7 @@ use crate::setup::installer::{ use crate::setup::registry::{DebuggerInfo, Platform}; use crate::setup::verifier::{verify_dap_adapter_tcp, VerifyResult}; use async_trait::async_trait; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; static INFO: DebuggerInfo = DebuggerInfo { id: "js-debug", @@ -98,7 +98,7 @@ impl Installer for JsDebugInstaller { } } -fn get_dap_executable(adapter_dir: &PathBuf) -> PathBuf { +fn get_dap_executable(adapter_dir: &Path) -> PathBuf { // @vscode/js-debug installs to node_modules/@vscode/js-debug let js_path = adapter_dir.join("node_modules/@vscode/js-debug/src/dapDebugServer.js"); if js_path.exists() { @@ -107,7 +107,7 @@ fn get_dap_executable(adapter_dir: &PathBuf) -> PathBuf { adapter_dir.join("node_modules/@vscode/js-debug/dist/src/dapDebugServer.js") } -fn read_package_version(adapter_dir: &PathBuf) -> Option { +fn read_package_version(adapter_dir: &Path) -> Option { let package_json = adapter_dir.join("node_modules/@vscode/js-debug/package.json"); if !package_json.exists() { return None; diff --git a/src/setup/adapters/lldb.rs b/src/setup/adapters/lldb.rs index f12440a..5cb4456 100644 --- a/src/setup/adapters/lldb.rs +++ b/src/setup/adapters/lldb.rs @@ -260,7 +260,7 @@ async fn install_from_github(opts: &InstallOptions) -> Result { let platform = platform_str(); let arch = arch_str(); - let asset_patterns = vec![ + let asset_patterns = [ format!("LLVM-*-{}-{}.tar.xz", arch, platform), format!("clang+llvm-*-{}-*{}.tar.xz", arch, platform), ]; diff --git a/src/setup/detector.rs b/src/setup/detector.rs index c8a45fc..9b52859 100644 --- a/src/setup/detector.rs +++ b/src/setup/detector.rs @@ -121,13 +121,10 @@ fn has_c_files(dir: &Path) -> bool { /// Check if directory contains files with a specific extension fn has_extension_in_dir(dir: &Path, ext: &str) -> bool { if let Ok(entries) = std::fs::read_dir(dir) { - for entry in entries { - // Explicitly handle Result - skip entries that can't be read - if let Ok(entry) = entry { - let path = entry.path(); - if path.extension().map(|e| e == ext).unwrap_or(false) { - return true; - } + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().map(|e| e == ext).unwrap_or(false) { + return true; } } } diff --git a/src/setup/verifier.rs b/src/setup/verifier.rs index d20b310..f082d06 100644 --- a/src/setup/verifier.rs +++ b/src/setup/verifier.rs @@ -164,7 +164,6 @@ pub async fn verify_dap_adapter_tcp( // Retry TCP connection with exponential backoff let stream = { - let mut last_error = String::new(); let mut delay = Duration::from_millis(100); let max_delay = Duration::from_millis(1000); let timeout_duration = Duration::from_secs(10); @@ -172,15 +171,14 @@ pub async fn verify_dap_adapter_tcp( loop { match TcpStream::connect(&addr).await { - Ok(s) => break s, - Err(e) => { - last_error = e.to_string(); - if start.elapsed() >= timeout_duration { + Ok(s) => break s, + Err(e) => { + if start.elapsed() >= timeout_duration { let _ = child.kill().await; return Ok(VerifyResult { success: false, capabilities: None, - error: Some(format!("Failed to connect to {} after {:?}: {}", addr, timeout_duration, last_error)), + error: Some(format!("Failed to connect to {} after {:?}: {}", addr, timeout_duration, e)), }); } tokio::time::sleep(delay).await; diff --git a/src/testing/config.rs b/src/testing/config.rs index adf3dcf..86f0275 100644 --- a/src/testing/config.rs +++ b/src/testing/config.rs @@ -103,6 +103,10 @@ pub struct CommandExpectation { pub success: Option, /// Substring that should be in the output pub output_contains: Option, + /// Permit either command outcome. Unlike `success: false`, this does not + /// require a failure and is intended only for documented adapter variance. + #[serde(default)] + pub allow_failure: bool, } /// Expectations for a stop event diff --git a/src/testing/runner.rs b/src/testing/runner.rs index cc06942..a750a91 100644 --- a/src/testing/runner.rs +++ b/src/testing/runner.rs @@ -175,7 +175,7 @@ pub async fn run_scenario(path: &Path, verbose: bool) -> Result { } println!(" {} Attached to process", "✓".green()); - } else if scenario.target.mode != "launch" && scenario.target.mode != "launch" { + } else if scenario.target.mode != "launch" { // Unknown mode - fail explicitly return Err(Error::Config(format!( "Unknown target mode '{}'. Supported modes: 'launch', 'attach'", @@ -301,9 +301,16 @@ async fn execute_command_step( let cmd = parse_command(command_str)?; let result = client.send_command(cmd).await; + let allow_failure = expect.map(|exp| exp.allow_failure).unwrap_or(false); // Check expectations if let Some(exp) = expect { + if exp.allow_failure && exp.success.is_some() { + return Err(Error::Config(format!( + "Command '{}' cannot combine allow_failure with success", + command_str + ))); + } if let Some(should_succeed) = exp.success { let did_succeed = result.is_ok(); if should_succeed != did_succeed { @@ -326,7 +333,35 @@ async fn execute_command_step( return Ok(()); } - result?; + let value = match result { + Ok(value) => value, + Err(error) if allow_failure => { + println!( + " {} Step {}: {} (allowed failure: {})", + "✓".green(), + step_num, + command_str.dimmed(), + error.to_string().dimmed() + ); + return Ok(()); + } + Err(error) => return Err(error), + }; + + if let Some(expected_substr) = expect.and_then(|exp| exp.output_contains.as_ref()) { + let output = serde_json::to_string(&value).map_err(|error| { + Error::TestAssertion(format!( + "Failed to serialize result of command '{}': {}", + command_str, error + )) + })?; + if !output.contains(expected_substr) { + return Err(Error::TestAssertion(format!( + "Command '{}' output missing '{}'", + command_str, expected_substr + ))); + } + } println!( " {} Step {}: {}", @@ -559,6 +594,7 @@ async fn execute_check_output_step( .send_command(Command::GetOutput { tail: None, clear: false, + cursor: None, }) .await?; @@ -725,45 +761,14 @@ fn parse_command(s: &str) -> Result { "break command requires a location".to_string(), )); } - // Handle "break add " or just "break " - // Also handle --condition "expr" flag - let mut location_str = String::new(); - let mut condition: Option = None; - let mut i = 0; - // Skip "add" subcommand if present AND there are more args // (otherwise "add" is the function name to break on) - if args.get(0) == Some(&"add") && args.len() > 1 { - i = 1; - } - - while i < args.len() { - if args[i] == "--condition" && i + 1 < args.len() { - // Collect condition expression (may be quoted) - i += 1; - let mut cond_parts = Vec::new(); - while i < args.len() && !args[i].starts_with("--") { - cond_parts.push(args[i]); - i += 1; - } - condition = Some(cond_parts.join(" ").trim_matches('"').to_string()); - } else if !args[i].starts_with("--") { - if !location_str.is_empty() { - location_str.push(' '); - } - location_str.push_str(args[i]); - i += 1; - } else { - i += 1; - } - } - - let location = BreakpointLocation::parse(&location_str)?; - Ok(Command::BreakpointAdd { - location, - condition, - hit_count: None, - }) + let breakpoint_args = if args.first() == Some(&"add") && args.len() > 1 { + &args[1..] + } else { + args + }; + parse_breakpoint_add(breakpoint_args, "break") } "breakpoint" => { @@ -780,12 +785,7 @@ fn parse_command(s: &str) -> Result { "breakpoint add requires a location".to_string(), )); } - let location = BreakpointLocation::parse(args[1])?; - Ok(Command::BreakpointAdd { - location, - condition: None, - hit_count: None, - }) + parse_breakpoint_add(&args[1..], "breakpoint add") } "remove" => { if args.len() < 2 { @@ -832,6 +832,25 @@ fn parse_command(s: &str) -> Result { } } + "context" | "where" => { + let lines = match args { + [] => 5, + [value] => value.parse().map_err(|_| { + Error::Config(format!("{} requires a numeric line count", cmd)) + })?, + [flag, value] if *flag == "--lines" => value.parse().map_err(|_| { + Error::Config(format!("{} --lines requires a number", cmd)) + })?, + _ => { + return Err(Error::Config(format!( + "{} accepts either or --lines ", + cmd + ))) + } + }; + Ok(Command::Context { lines }) + } + "locals" => Ok(Command::Locals { frame_id: None }), "backtrace" | "bt" => Ok(Command::StackTrace { @@ -875,7 +894,11 @@ fn parse_command(s: &str) -> Result { Ok(Command::Evaluate { expression: args.join(" "), frame_id: None, - context: EvaluateContext::Watch, + context: if cmd == "eval" { + EvaluateContext::Repl + } else { + EvaluateContext::Watch + }, }) } @@ -884,36 +907,98 @@ fn parse_command(s: &str) -> Result { "restart" => Ok(Command::Restart), "output" => { - // Parse --tail N and --clear flags + // Parse the same options accepted by the user-facing CLI. let mut tail: Option = None; let mut clear = false; let mut i = 0; while i < args.len() { match args[i] { - "--tail" => { - if i + 1 < args.len() { - tail = args[i + 1].parse().ok(); - i += 2; - } else { - i += 1; - } + "--tail" | "-t" => { + let value = args.get(i + 1).ok_or_else(|| { + Error::Config("output --tail requires a number".to_string()) + })?; + tail = Some(value.parse().map_err(|_| { + Error::Config(format!("Invalid output tail value: {}", value)) + })?); + i += 2; } "--clear" => { clear = true; i += 1; } - _ => { - i += 1; + option => { + return Err(Error::Config(format!( + "Unknown output option: {}", + option + ))); } } } - Ok(Command::GetOutput { tail, clear }) + Ok(Command::GetOutput { tail, clear, cursor: None }) } _ => Err(Error::Config(format!("Unknown command: {}", cmd))), } } +/// Parse a breakpoint location and the shared breakpoint options used by the +/// CLI shorthand and the `breakpoint add` subcommand. +fn parse_breakpoint_add(args: &[&str], command: &str) -> Result { + let mut location_parts = Vec::new(); + let mut condition = None; + let mut hit_count = None; + let mut index = 0; + + while index < args.len() { + match args[index] { + "--condition" | "-c" => { + index += 1; + let mut condition_parts = Vec::new(); + while index < args.len() && !args[index].starts_with("--") { + condition_parts.push(args[index]); + index += 1; + } + if condition_parts.is_empty() { + return Err(Error::Config(format!( + "{} --condition requires an expression", + command + ))); + } + condition = Some(condition_parts.join(" ").trim_matches('"').to_string()); + } + "--hit-count" => { + let value = args.get(index + 1).ok_or_else(|| { + Error::Config(format!("{} --hit-count requires a number", command)) + })?; + hit_count = Some(value.parse().map_err(|_| { + Error::Config(format!("Invalid hit count: {}", value)) + })?); + index += 2; + } + option if option.starts_with('-') => { + return Err(Error::Config(format!( + "Unknown {} option: {}", + command, option + ))); + } + location => { + location_parts.push(location); + index += 1; + } + } + } + + if location_parts.is_empty() { + return Err(Error::Config(format!("{} requires a location", command))); + } + + Ok(Command::BreakpointAdd { + location: BreakpointLocation::parse(&location_parts.join(" "))?, + condition, + hit_count, + }) +} + #[cfg(test)] mod tests { use super::*; @@ -960,10 +1045,101 @@ mod tests { fn test_parse_print_commands() { let cmd = parse_command("print x + y").unwrap(); match cmd { - Command::Evaluate { expression, .. } => { + Command::Evaluate { + expression, context, .. + } => { assert_eq!(expression, "x + y"); + assert!(matches!(context, EvaluateContext::Watch)); } _ => panic!("Expected Evaluate command"), } + + let cmd = parse_command("eval counter = counter + 1").unwrap(); + assert!(matches!( + cmd, + Command::Evaluate { + context: EvaluateContext::Repl, + .. + } + )); + } + + #[test] + fn test_parse_break_with_hit_count() { + let cmd = parse_command("break factorial --hit-count 3").unwrap(); + match cmd { + Command::BreakpointAdd { hit_count, .. } => { + assert_eq!(hit_count, Some(3)); + } + _ => panic!("Expected BreakpointAdd command"), + } + + let cmd = parse_command("break main.c:10 --hit-count 5").unwrap(); + match cmd { + Command::BreakpointAdd { hit_count, .. } => { + assert_eq!(hit_count, Some(5)); + } + _ => panic!("Expected BreakpointAdd command"), + } + } + + #[test] + fn test_parse_break_with_condition_and_hit_count() { + let cmd = parse_command("break foo --condition \"x > 5\" --hit-count 2").unwrap(); + match cmd { + Command::BreakpointAdd { condition, hit_count, .. } => { + assert_eq!(condition, Some("x > 5".to_string())); + assert_eq!(hit_count, Some(2)); + } + _ => panic!("Expected BreakpointAdd command"), + } + } + + #[test] + fn test_parse_breakpoint_subcommand_options() { + let cmd = parse_command("breakpoint add foo.c:10 --condition x > 5 --hit-count 2") + .unwrap(); + match cmd { + Command::BreakpointAdd { + condition, + hit_count, + .. + } => { + assert_eq!(condition, Some("x > 5".to_string())); + assert_eq!(hit_count, Some(2)); + } + _ => panic!("Expected BreakpointAdd command"), + } + } + + #[test] + fn test_parse_context_commands() { + assert!(matches!( + parse_command("context").unwrap(), + Command::Context { lines: 5 } + )); + assert!(matches!( + parse_command("where 3").unwrap(), + Command::Context { lines: 3 } + )); + assert!(matches!( + parse_command("context --lines 7").unwrap(), + Command::Context { lines: 7 } + )); + assert!(parse_command("context --lines nope").is_err()); + } + + #[test] + fn test_parse_output_commands() { + assert!(matches!( + parse_command("output -t 4 --clear").unwrap(), + Command::GetOutput { + tail: Some(4), + clear: true, + cursor: None + } + )); + assert!(parse_command("output --tail invalid").is_err()); + assert!(parse_command("output --follow").is_err()); } } diff --git a/tests/TESTING.md b/tests/TESTING.md index 078ce74..a6573fc 100644 --- a/tests/TESTING.md +++ b/tests/TESTING.md @@ -11,8 +11,8 @@ debugger test tests/scenarios/hello_world_c.yml # Run with verbose output debugger test tests/scenarios/hello_world_c.yml --verbose -# Run with a specific adapter -debugger test tests/scenarios/hello_world_c.yml --adapter gdb +# Select an adapter in the scenario's `target.adapter` field. +debugger test tests/scenarios/hello_world_c.yml ``` ## Test Architecture @@ -107,9 +107,11 @@ steps: debugger test tests/scenarios/your_new_test.yml --verbose ``` -### Step 4: Add to CI +### Step 4: Run It With Its Adapter -Add the test to `.github/workflows/e2e-tests.yml` under the appropriate adapter job. +Adapter-backed scenarios are opt-in: run the scenario on a machine with its +declared adapter installed. The repository CI always runs the Rust test suite +and Clippy, but it does not provision every DAP adapter. ## Step Types Reference @@ -191,9 +193,8 @@ command: "break add" - Test basic debugging workflow - Set breakpoint, continue, inspect locals, exit -3. **Add compilation to CI**: `.github/workflows/e2e-tests.yml` - - Add fixture compilation step - - Add adapter-specific test job if needed +3. **Document the adapter prerequisite** in the scenario description and run + it locally before submitting changes. 4. **Update documentation**: - `tests/fixtures/README.md` - Document new fixture @@ -214,12 +215,6 @@ Not all features work with all adapters: ## Running Tests in CI -Tests run automatically on push/PR via GitHub Actions: -- Matrix: 5 adapters × 2 platforms -- Tests have automatic retry (3 attempts) for flaky test handling -- Failed test logs uploaded as artifacts - -To debug CI failures: -1. Check the job logs for error messages -2. Download log artifacts from the failed run -3. Reproduce locally with `--verbose` flag +The GitHub Actions workflow runs `cargo test --all-targets --no-fail-fast` and +`cargo clippy --all-targets -- -D warnings` on every push and pull request. +Adapter-specific scenarios should be reproduced locally with `--verbose`. diff --git a/tests/e2e/test_rs b/tests/e2e/test_rs deleted file mode 100755 index ebb8dde..0000000 Binary files a/tests/e2e/test_rs and /dev/null differ diff --git a/tests/fixtures/test_simple_go b/tests/fixtures/test_simple_go deleted file mode 100755 index 395cf56..0000000 Binary files a/tests/fixtures/test_simple_go and /dev/null differ diff --git a/tests/integration.rs b/tests/integration.rs index fa18908..34fd1eb 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -8,7 +8,6 @@ use std::collections::HashMap; use std::env; use std::fs; -use std::io::{BufRead, BufReader}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::time::Duration; @@ -92,27 +91,6 @@ impl TestContext { self.binaries.get(name).unwrap() } - /// Build a Rust fixture - fn build_rust_fixture(&mut self, name: &str) -> &PathBuf { - let source = self.fixtures_dir.join(format!("{}.rs", name)); - let output = self.temp_dir.join(format!("{}_rs", name)); - - let status = Command::new("rustc") - .args([ - "-g", // Debug symbols - "-o", - output.to_str().unwrap(), - source.to_str().unwrap(), - ]) - .status() - .expect("Failed to compile Rust fixture"); - - assert!(status.success(), "Rust compilation failed"); - - self.binaries.insert(format!("{}_rs", name), output.clone()); - self.binaries.get(&format!("{}_rs", name)).unwrap() - } - /// Find breakpoint line numbers from markers in source fn find_breakpoint_markers(&self, source: &Path) -> HashMap { let content = fs::read_to_string(source).expect("Failed to read source file"); @@ -121,7 +99,6 @@ impl TestContext { for (line_num, line) in content.lines().enumerate() { if let Some(marker_start) = line.find("BREAKPOINT_MARKER:") { let marker_name = line[marker_start + "BREAKPOINT_MARKER:".len()..] - .trim() .split_whitespace() .next() .unwrap() @@ -227,12 +204,20 @@ max_bytes_mb = 1 fs::write(&config_path, config_content).expect("Failed to write config"); } - /// Run a debugger command - fn run_debugger(&self, args: &[&str]) -> DebuggerOutput { - let output = Command::new(&self.debugger_bin) + /// Build a debugger command scoped to this test's daemon and configuration. + fn debugger_command(&self, args: &[&str]) -> Command { + let mut command = Command::new(&self.debugger_bin); + command .args(args) .env("XDG_CONFIG_HOME", &self.config_dir) - .env("XDG_RUNTIME_DIR", &self.runtime_dir) + .env("XDG_RUNTIME_DIR", &self.runtime_dir); + command + } + + /// Run a debugger command. + fn run_debugger(&self, args: &[&str]) -> DebuggerOutput { + let output = self + .debugger_command(args) .output() .expect("Failed to run debugger"); @@ -240,7 +225,6 @@ max_bytes_mb = 1 stdout: String::from_utf8_lossy(&output.stdout).to_string(), stderr: String::from_utf8_lossy(&output.stderr).to_string(), success: output.status.success(), - code: output.status.code(), } } @@ -289,7 +273,6 @@ struct DebuggerOutput { stdout: String, stderr: String, success: bool, - code: Option, } /// Find the debugger binary @@ -528,7 +511,6 @@ fn test_basic_debugging_workflow_c() { } #[test] -#[ignore = "GDB DAP mode has different stopOnEntry behavior than LLDB"] fn test_basic_debugging_workflow_c_gdb() { let gdb_path = match gdb_available() { Some(path) => path, @@ -546,29 +528,61 @@ fn test_basic_debugging_workflow_c_gdb() { // Find breakpoint markers let markers = ctx.find_breakpoint_markers(&ctx.fixtures_dir.join("simple.c")); - let main_start_line = markers.get("main_start").expect("Missing main_start marker"); + let add_body_line = markers.get("add_body").expect("Missing add_body marker"); // Cleanup any existing daemon ctx.cleanup_daemon(); - // Start debugging - let output = ctx.run_debugger_ok(&[ - "start", - binary.to_str().unwrap(), - "--stop-on-entry", - ]); + // GDB's DAP launch response arrives only after configurationDone, so this + // specifically exercises a breakpoint configured during startup. + let breakpoint = format!("simple.c:{}", add_body_line); + let output = ctx.run_debugger_ok(&["start", binary.to_str().unwrap(), "--break", &breakpoint]); assert!(output.contains("Started debugging") || output.contains("Stopped")); - // Set a breakpoint - let bp_location = format!("simple.c:{}", main_start_line); - let output = ctx.run_debugger_ok(&["break", &bp_location]); - assert!(output.contains("Breakpoint") || output.contains("breakpoint")); + let output = ctx.run_debugger_ok(&["breakpoint", "list"]); + assert!( + output.contains("simple.c"), + "Initial breakpoint should be tracked: {}", + output + ); - // Continue execution - let output = ctx.run_debugger_ok(&["continue"]); - assert!(output.contains("Continuing") || output.contains("running")); + // A follower polls over short-lived connections. Verify it does not hold + // the daemon hostage while another client asks for status. + let mut follower = ctx + .debugger_command(&["output", "--follow"]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("Failed to start output follower"); + std::thread::sleep(Duration::from_millis(300)); + + let mut status = ctx + .debugger_command(&["status"]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("Failed to start status command"); + let deadline = std::time::Instant::now() + Duration::from_secs(2); + while status.try_wait().expect("Failed to poll status command").is_none() { + if std::time::Instant::now() >= deadline { + let _ = status.kill(); + let _ = follower.kill(); + panic!("output --follow blocked another daemon client"); + } + std::thread::sleep(Duration::from_millis(20)); + } + let status_output = status + .wait_with_output() + .expect("Failed to collect status output"); + assert!( + status_output.status.success(), + "Status failed while following output: {}", + String::from_utf8_lossy(&status_output.stderr) + ); + let _ = follower.kill(); + let _ = follower.wait(); - // Wait for breakpoint hit + // Wait for the initial breakpoint to be hit. let output = ctx.run_debugger_ok(&["await", "--timeout", "30"]); assert!( output.contains("Stopped") || output.contains("breakpoint"), @@ -576,7 +590,7 @@ fn test_basic_debugging_workflow_c_gdb() { output ); - // Get local variables + // Get local variables and evaluate in the selected frame. let output = ctx.run_debugger_ok(&["locals"]); assert!( output.contains("x") || output.contains("Local"), @@ -584,6 +598,30 @@ fn test_basic_debugging_workflow_c_gdb() { output ); + let output = ctx.run_debugger_ok(&["print", "a + b"]); + assert!(output.contains("30"), "Expected a+b=30: {}", output); + + // Context must honor the frame selected by `up`; this used to be reset to + // frame 0 by a stack-trace request. + let output = ctx.run_debugger_ok(&["up"]); + assert!(output.contains("main"), "Expected caller frame: {}", output); + let output = ctx.run_debugger_ok(&["context", "--lines", "2"]); + assert!(output.contains("function: main") || output.contains("In function: main")); + ctx.run_debugger_ok(&["down"]); + + // Finish the program and verify that buffered debuggee output is available. + ctx.run_debugger_ok(&["continue"]); + let output = ctx.run_debugger_ok(&["await", "--timeout", "30"]); + assert!(output.contains("exited") || output.contains("terminated")); + let output = ctx.run_debugger_ok(&["output"]); + assert!(output.contains("Sum: 30"), "Expected program output: {}", output); + let output = ctx.run_debugger_ok(&["output", "--tail", "1"]); + assert!( + output.contains("Factorial: 120") && !output.contains("Sum: 30"), + "Expected line-based output tail: {}", + output + ); + // Stop the session let _ = ctx.run_debugger(&["stop"]); } @@ -716,7 +754,9 @@ fn test_multiple_breakpoints_c() { // Set multiple breakpoints for marker in ["main_start", "before_add", "before_factorial"] { - let line = markers.get(marker).expect(&format!("Missing {} marker", marker)); + let line = markers + .get(marker) + .unwrap_or_else(|| panic!("Missing {} marker", marker)); let bp_location = format!("simple.c:{}", line); ctx.run_debugger_ok(&["break", &bp_location]); } @@ -1175,3 +1215,86 @@ fn test_expression_evaluation_js() { ctx.run_debugger(&["stop"]); } + +#[test] +fn test_await_does_not_block_other_clients_gdb() { + let gdb_path = match gdb_available() { + Some(path) => path, + None => { + eprintln!("Skipping test: GDB ≥14.1 not available"); + return; + } + }; + + let mut ctx = TestContext::new("await_concurrent_gdb"); + ctx.create_config_with_args("gdb", gdb_path.to_str().unwrap(), &["-i=dap"]); + + // Long-running fixture keeps the program running while await blocks. + let binary = ctx.build_c_fixture("attach_target").clone(); + + ctx.cleanup_daemon(); + + let output = ctx.run_debugger_ok(&["start", binary.to_str().unwrap()]); + assert!(output.contains("Started debugging") || output.contains("running")); + + // Block one client in await, then pause from a second client. Before the + // session actor, the awaiting connection owned the daemon, so pause could + // not even be received until await timed out. + let mut awaiter = ctx + .debugger_command(&["await", "--timeout", "20"]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("Failed to start await command"); + std::thread::sleep(Duration::from_millis(500)); + + let mut pause = ctx + .debugger_command(&["pause"]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("Failed to start pause command"); + let deadline = std::time::Instant::now() + Duration::from_secs(5); + while pause.try_wait().expect("Failed to poll pause command").is_none() { + if std::time::Instant::now() >= deadline { + let _ = pause.kill(); + let _ = awaiter.kill(); + panic!("pause blocked behind an in-flight await"); + } + std::thread::sleep(Duration::from_millis(20)); + } + let pause_output = pause + .wait_with_output() + .expect("Failed to collect pause output"); + assert!( + pause_output.status.success(), + "Pause failed while another client awaited: {}", + String::from_utf8_lossy(&pause_output.stderr) + ); + + // The pause must wake the awaiting client with a stop, well before its + // 20-second timeout. + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while awaiter.try_wait().expect("Failed to poll await command").is_none() { + if std::time::Instant::now() >= deadline { + let _ = awaiter.kill(); + panic!("await did not observe the pause-induced stop"); + } + std::thread::sleep(Duration::from_millis(50)); + } + let await_output = awaiter + .wait_with_output() + .expect("Failed to collect await output"); + // Finishing within the deadline already proves await saw the stop rather + // than its own 20-second timeout; the stop reason wording varies by GDB + // version, so only require a successful, non-empty report. + let stdout = String::from_utf8_lossy(&await_output.stdout); + assert!( + await_output.status.success() && !stdout.trim().is_empty(), + "Expected await to report a stop: stdout={} stderr={}", + stdout, + String::from_utf8_lossy(&await_output.stderr) + ); + + ctx.run_debugger(&["stop"]); +} diff --git a/tests/scenarios/README.md b/tests/scenarios/README.md index 55bf12c..814d1d6 100644 --- a/tests/scenarios/README.md +++ b/tests/scenarios/README.md @@ -75,21 +75,16 @@ debugger test tests/scenarios/hello_world_c.yml # Verbose output debugger test tests/scenarios/conditional_breakpoint_go.yml --verbose -# Run with specific adapter -debugger test tests/scenarios/hello_world_c.yml --adapter gdb +# Select an adapter in the scenario's `target.adapter` field. +debugger test tests/scenarios/hello_world_c.yml ``` ## Running Tests in CI -GitHub Actions runs all scenarios across adapter/OS matrix: - -- **LLDB**: Ubuntu + macOS (C, Rust) -- **GDB**: Ubuntu + macOS (C) -- **Delve**: Ubuntu + macOS (Go) -- **debugpy**: Ubuntu + macOS (Python) -- **js-debug**: Ubuntu + macOS (JavaScript, TypeScript) - -Tests run on every push and PR via GitHub Actions. The workflow includes parallel jobs for each adapter (LLDB, GDB, Delve, debugpy, js-debug) on Ubuntu and macOS, with graceful fallback for macOS GDB installation failures. +The repository CI runs the Rust test suite and Clippy on every push and pull +request. Adapter-backed scenarios remain opt-in because they require the +relevant adapter and toolchain to be installed; run them locally before adding +or changing a scenario. ## Adapter Feature Compatibility @@ -103,7 +98,7 @@ Not all features work with all adapters. Tests are created only for compatible c | Stack navigation | ✅ | ✅ | ✅ | ✅ | ✅ | | Output capture | ✅ | ✅ | ✅ | ✅ | ✅ | | Pause | ✅ | ✅ | ✅ | ✅ | ✅ | -| Restart | ✅ | ✅ | ✅ | ✅ | ✅ | +| Restart | Adapter-dependent | Adapter-dependent | Adapter-dependent | Adapter-dependent | Adapter-dependent | ## Writing New Scenarios diff --git a/tests/scenarios/attach_process_c.yml b/tests/scenarios/attach_process_c.yml index c82e4ba..d22d4c8 100644 --- a/tests/scenarios/attach_process_c.yml +++ b/tests/scenarios/attach_process_c.yml @@ -15,7 +15,7 @@ setup: target: # Attach mode uses pid_file field to read PID from setup step - program: "tests/fixtures/test_attach_c" + program: "../fixtures/test_attach_c" mode: "attach" pid_file: "/tmp/attach_pid.txt" adapter: "lldb" diff --git a/tests/scenarios/breakpoint_management_c.yml b/tests/scenarios/breakpoint_management_c.yml index 99c1d43..8039fce 100644 --- a/tests/scenarios/breakpoint_management_c.yml +++ b/tests/scenarios/breakpoint_management_c.yml @@ -8,7 +8,7 @@ setup: - shell: "gcc -g tests/fixtures/simple.c -o tests/fixtures/test_simple_c" target: - program: "tests/fixtures/test_simple_c" + program: "../fixtures/test_simple_c" args: [] stop_on_entry: true @@ -46,6 +46,12 @@ steps: expect: reason: "breakpoint" + # Remove remaining breakpoints to avoid hitting them again + - action: command + command: "breakpoint remove all" + expect: + success: true + - action: command command: "continue" diff --git a/tests/scenarios/complex_verification.yml b/tests/scenarios/complex_verification.yml index cfe6b7a..2e4a613 100644 --- a/tests/scenarios/complex_verification.yml +++ b/tests/scenarios/complex_verification.yml @@ -10,7 +10,7 @@ setup: # Debug target configuration target: - program: "tests/fixtures/test_simple_c" + program: "../fixtures/test_simple_c" args: [] stop_on_entry: true diff --git a/tests/scenarios/conditional_breakpoint_js.yml b/tests/scenarios/conditional_breakpoint_js.yml index b23dd65..b93f61a 100644 --- a/tests/scenarios/conditional_breakpoint_js.yml +++ b/tests/scenarios/conditional_breakpoint_js.yml @@ -5,7 +5,7 @@ name: "JavaScript Conditional Breakpoint Test" description: "Verifies conditional breakpoints stop only when condition is true" target: - program: "tests/fixtures/simple.js" + program: "../fixtures/simple.js" args: [] adapter: "js-debug" stop_on_entry: true diff --git a/tests/scenarios/conditional_breakpoint_py.yml b/tests/scenarios/conditional_breakpoint_py.yml index 3e2bd69..0b9affd 100644 --- a/tests/scenarios/conditional_breakpoint_py.yml +++ b/tests/scenarios/conditional_breakpoint_py.yml @@ -5,7 +5,7 @@ name: "Python Conditional Breakpoint Test" description: "Verifies conditional breakpoints stop only when condition is true" target: - program: "tests/fixtures/simple.py" + program: "../fixtures/simple.py" args: [] adapter: "python" stop_on_entry: true diff --git a/tests/scenarios/error_attach_invalid_pid_c.yml b/tests/scenarios/error_attach_invalid_pid_c.yml new file mode 100644 index 0000000..75516d3 --- /dev/null +++ b/tests/scenarios/error_attach_invalid_pid_c.yml @@ -0,0 +1,23 @@ +# Attach Invalid PID Error Test +# Tests error handling when attaching to non-existent PID + +name: "C Attach Invalid PID Test" +description: "Verifies clear error when attaching to invalid PID" + +setup: + # Compile target but do NOT run it - no process to attach to + - shell: "gcc -g tests/fixtures/attach_target.c -o tests/fixtures/test_attach_c" + +target: + program: "tests/fixtures/test_attach_c" + mode: "attach" + # INT32_MAX exceeds OS PID limits on all platforms - guaranteed invalid + pid: 2147483647 + adapter: "lldb" + +steps: + # This step should fail during session startup with clear error message + - action: command + command: "continue" + expect: + success: false diff --git a/tests/scenarios/error_bad_expression_c.yml b/tests/scenarios/error_bad_expression_c.yml index e34e6c8..29f67ce 100644 --- a/tests/scenarios/error_bad_expression_c.yml +++ b/tests/scenarios/error_bad_expression_c.yml @@ -8,7 +8,7 @@ setup: - shell: "gcc -g tests/fixtures/simple.c -o tests/fixtures/test_simple_c" target: - program: "tests/fixtures/test_simple_c" + program: "../fixtures/test_simple_c" adapter: "lldb" stop_on_entry: true diff --git a/tests/scenarios/error_invalid_breakpoint_c.yml b/tests/scenarios/error_invalid_breakpoint_c.yml index 379b595..4e50794 100644 --- a/tests/scenarios/error_invalid_breakpoint_c.yml +++ b/tests/scenarios/error_invalid_breakpoint_c.yml @@ -8,7 +8,7 @@ setup: - shell: "gcc -g tests/fixtures/simple.c -o tests/fixtures/test_simple_c" target: - program: "tests/fixtures/test_simple_c" + program: "../fixtures/test_simple_c" adapter: "lldb" stop_on_entry: true diff --git a/tests/scenarios/error_undefined_variable_c.yml b/tests/scenarios/error_undefined_variable_c.yml index 25c3ef7..8673a2c 100644 --- a/tests/scenarios/error_undefined_variable_c.yml +++ b/tests/scenarios/error_undefined_variable_c.yml @@ -8,7 +8,7 @@ setup: - shell: "gcc -g tests/fixtures/simple.c -o tests/fixtures/test_simple_c" target: - program: "tests/fixtures/test_simple_c" + program: "../fixtures/test_simple_c" adapter: "lldb" stop_on_entry: true diff --git a/tests/scenarios/expression_eval_js.yml b/tests/scenarios/expression_eval_js.yml index efa56cb..4c520e5 100644 --- a/tests/scenarios/expression_eval_js.yml +++ b/tests/scenarios/expression_eval_js.yml @@ -6,7 +6,7 @@ description: "Verifies expression evaluation in debug context" # Debug target configuration target: - program: "tests/fixtures/simple.js" + program: "../fixtures/simple.js" args: [] adapter: "js-debug" stop_on_entry: true diff --git a/tests/scenarios/hello_world_c.yml b/tests/scenarios/hello_world_c.yml index c38160c..9fe1349 100644 --- a/tests/scenarios/hello_world_c.yml +++ b/tests/scenarios/hello_world_c.yml @@ -10,7 +10,7 @@ setup: # Debug target configuration target: - program: "tests/e2e/test_c" + program: "../e2e/test_c" args: [] stop_on_entry: true diff --git a/tests/scenarios/hello_world_js.yml b/tests/scenarios/hello_world_js.yml index aa2fbd1..9cf5db2 100644 --- a/tests/scenarios/hello_world_js.yml +++ b/tests/scenarios/hello_world_js.yml @@ -6,7 +6,7 @@ description: "Verifies basic JavaScript debugging functionality with Node.js" # Debug target configuration target: - program: "tests/e2e/hello_world.js" + program: "../e2e/hello_world.js" args: [] adapter: "js-debug" stop_on_entry: true diff --git a/tests/scenarios/hello_world_python.yml b/tests/scenarios/hello_world_python.yml index 28d4b85..471b44b 100644 --- a/tests/scenarios/hello_world_python.yml +++ b/tests/scenarios/hello_world_python.yml @@ -8,7 +8,7 @@ description: "Verifies basic Python debugging functionality" # Debug target configuration target: - program: "tests/e2e/hello_world.py" + program: "../e2e/hello_world.py" args: [] adapter: "debugpy" stop_on_entry: true diff --git a/tests/scenarios/hello_world_rust.yml b/tests/scenarios/hello_world_rust.yml index c4793fa..ef8bbb7 100644 --- a/tests/scenarios/hello_world_rust.yml +++ b/tests/scenarios/hello_world_rust.yml @@ -10,7 +10,7 @@ setup: # Debug target configuration target: - program: "tests/e2e/test_rs" + program: "../e2e/test_rs" args: [] stop_on_entry: true @@ -48,11 +48,17 @@ steps: - index: 0 function: "main" - # 7. Continue to exit + # 7. Remove breakpoints to avoid hitting them again + - action: command + command: "breakpoint remove all" + expect: + success: true + + # 8. Continue to exit - action: command command: "continue" - # 8. Wait for program exit + # 9. Wait for program exit - action: await timeout: 10 expect: diff --git a/tests/scenarios/hello_world_ts.yml b/tests/scenarios/hello_world_ts.yml index 094c909..c2addc8 100644 --- a/tests/scenarios/hello_world_ts.yml +++ b/tests/scenarios/hello_world_ts.yml @@ -11,7 +11,7 @@ setup: # Debug target configuration # Note: We debug the compiled JS but set breakpoints in TS source target: - program: "tests/e2e/dist/hello_world.js" + program: "../e2e/dist/hello_world.js" args: [] adapter: "js-debug" stop_on_entry: true diff --git a/tests/scenarios/hitcount_breakpoint_c.yml b/tests/scenarios/hitcount_breakpoint_c.yml index 2299167..c86cc0b 100644 --- a/tests/scenarios/hitcount_breakpoint_c.yml +++ b/tests/scenarios/hitcount_breakpoint_c.yml @@ -8,7 +8,7 @@ setup: - shell: "gcc -g tests/fixtures/simple.c -o tests/fixtures/test_simple_c" target: - program: "tests/fixtures/test_simple_c" + program: "../fixtures/test_simple_c" args: [] stop_on_entry: true @@ -31,6 +31,12 @@ steps: - index: 0 function: "factorial" + # Remove breakpoints to avoid hitting them again during remaining recursive calls + - action: command + command: "breakpoint remove all" + expect: + success: true + - action: command command: "continue" diff --git a/tests/scenarios/hitcount_breakpoint_go.yml b/tests/scenarios/hitcount_breakpoint_go.yml index 1a6b746..a6ff7da 100644 --- a/tests/scenarios/hitcount_breakpoint_go.yml +++ b/tests/scenarios/hitcount_breakpoint_go.yml @@ -32,6 +32,12 @@ steps: - index: 0 function: "main.factorial" + # Remove breakpoints to avoid hitting them again during remaining recursive calls + - action: command + command: "breakpoint remove all" + expect: + success: true + - action: command command: "continue" diff --git a/tests/scenarios/output_capture_c.yml b/tests/scenarios/output_capture_c.yml index d8c7e19..5ac66d3 100644 --- a/tests/scenarios/output_capture_c.yml +++ b/tests/scenarios/output_capture_c.yml @@ -8,7 +8,7 @@ setup: - shell: "gcc -g tests/fixtures/simple.c -o tests/fixtures/test_simple_c" target: - program: "tests/fixtures/test_simple_c" + program: "../fixtures/test_simple_c" args: [] stop_on_entry: true diff --git a/tests/scenarios/output_capture_js.yml b/tests/scenarios/output_capture_js.yml index febfef4..e362c7a 100644 --- a/tests/scenarios/output_capture_js.yml +++ b/tests/scenarios/output_capture_js.yml @@ -5,7 +5,7 @@ name: "JavaScript Output Capture Test" description: "Verifies output command captures stdout correctly" target: - program: "tests/fixtures/simple.js" + program: "../fixtures/simple.js" args: [] adapter: "js-debug" stop_on_entry: true diff --git a/tests/scenarios/output_capture_py.yml b/tests/scenarios/output_capture_py.yml index c8aa36a..c4e1c83 100644 --- a/tests/scenarios/output_capture_py.yml +++ b/tests/scenarios/output_capture_py.yml @@ -5,7 +5,7 @@ name: "Python Output Capture Test" description: "Verifies output command captures stdout correctly" target: - program: "tests/fixtures/simple.py" + program: "../fixtures/simple.py" args: [] adapter: "python" stop_on_entry: true diff --git a/tests/scenarios/pause_resume_c.yml b/tests/scenarios/pause_resume_c.yml index 1828793..f7065d9 100644 --- a/tests/scenarios/pause_resume_c.yml +++ b/tests/scenarios/pause_resume_c.yml @@ -1,51 +1,52 @@ # Pause/Resume Test (C) # Tests pause command to stop running program -# Uses factorial breakpoint to ensure program runs long enough for reliable pause +# Uses attach_target fixture which runs long enough to reliably pause name: "C Pause/Resume Test" description: "Verifies pause command stops a running program" setup: - - shell: "gcc -g tests/fixtures/simple.c -o tests/fixtures/test_simple_c" + - shell: "gcc -g tests/fixtures/attach_target.c -o tests/fixtures/test_attach_target" target: - program: "tests/fixtures/test_simple_c" + program: "../fixtures/test_attach_target" args: [] stop_on_entry: true steps: - # Set breakpoint in factorial to ensure we can pause during computation + # Set breakpoint in main loop to verify we can reach that code - action: command - command: "break factorial" + command: "break main" expect: success: true - action: command command: "continue" - # Wait for first factorial hit + # Wait for breakpoint in main - action: await timeout: 10 expect: reason: "breakpoint" - # Remove breakpoint and continue - program will run through factorial(5) + # Remove all breakpoints and continue - program will start its 30-second loop - action: command - command: "breakpoint remove --all" + command: "breakpoint remove all" + expect: + success: true - action: command command: "continue" - # Immediately pause - factorial recursion should still be running + # Pause the running program - action: command command: "pause" expect: success: true + # Wait for stop - reason may be 'pause' or 'exception' depending on platform - action: await - timeout: 5 - expect: - reason: "pause" + timeout: 10 # Verify we're stopped somewhere in the program - action: command @@ -53,10 +54,8 @@ steps: expect: success: true + # Stop the debug session (don't wait for 30-second program to complete) - action: command - command: "continue" - - - action: await - timeout: 10 + command: "stop" expect: - reason: "exited" + success: true diff --git a/tests/scenarios/program_restart_c.yml b/tests/scenarios/program_restart_c.yml index 2dbc755..b7375b7 100644 --- a/tests/scenarios/program_restart_c.yml +++ b/tests/scenarios/program_restart_c.yml @@ -8,7 +8,7 @@ setup: - shell: "gcc -g tests/fixtures/simple.c -o tests/fixtures/test_simple_c" target: - program: "tests/fixtures/test_simple_c" + program: "../fixtures/test_simple_c" args: [] stop_on_entry: true @@ -31,6 +31,14 @@ steps: expect: success: true + # After restart, wait for any stop event (may be entry, breakpoint, or exception depending on platform) + - action: await + timeout: 10 + + # Continue to ensure we hit the breakpoint at main + - action: command + command: "continue" + - action: await timeout: 10 expect: @@ -41,6 +49,12 @@ steps: - index: 0 function: "main" + # Remove breakpoints before final continue + - action: command + command: "breakpoint remove all" + expect: + success: true + - action: command command: "continue" diff --git a/tests/scenarios/stack_navigation_js.yml b/tests/scenarios/stack_navigation_js.yml index bc4a89b..503916d 100644 --- a/tests/scenarios/stack_navigation_js.yml +++ b/tests/scenarios/stack_navigation_js.yml @@ -5,7 +5,7 @@ name: "JavaScript Stack Navigation Test" description: "Verifies stack frame navigation works correctly" target: - program: "tests/fixtures/simple.js" + program: "../fixtures/simple.js" args: [] adapter: "js-debug" stop_on_entry: true diff --git a/tests/scenarios/stack_navigation_py.yml b/tests/scenarios/stack_navigation_py.yml index a5627c2..24dfc6f 100644 --- a/tests/scenarios/stack_navigation_py.yml +++ b/tests/scenarios/stack_navigation_py.yml @@ -5,7 +5,7 @@ name: "Python Stack Navigation Test" description: "Verifies stack frame navigation works correctly" target: - program: "tests/fixtures/simple.py" + program: "../fixtures/simple.py" args: [] adapter: "python" stop_on_entry: true diff --git a/tests/scenarios/stepping_js.yml b/tests/scenarios/stepping_js.yml index 30de719..038a3f9 100644 --- a/tests/scenarios/stepping_js.yml +++ b/tests/scenarios/stepping_js.yml @@ -6,7 +6,7 @@ description: "Verifies stepping commands work correctly in JavaScript" # Debug target configuration target: - program: "tests/fixtures/simple.js" + program: "../fixtures/simple.js" args: [] adapter: "js-debug" stop_on_entry: true diff --git a/tests/scenarios/thread_list_c.yml b/tests/scenarios/thread_list_c.yml index eae8e64..0835b0f 100644 --- a/tests/scenarios/thread_list_c.yml +++ b/tests/scenarios/thread_list_c.yml @@ -8,7 +8,7 @@ setup: - shell: "gcc -g -pthread tests/fixtures/threaded.c -o tests/fixtures/test_threaded_c" target: - program: "tests/fixtures/test_threaded_c" + program: "../fixtures/test_threaded_c" args: [] stop_on_entry: true