fix: making pre/post_exec to use correct observers in case of withObserver

[SWASTIC-7]

Description

pre/post_exec were calling inner Observers, which caused discrepancy when using withObserver as pointed on issue #2440
fixed this by making pre/post_exec to exclusively call withObserver's observers when commandExecutor is run.

• Added test

Checklist

• I have run ./scripts/precommit.sh and addressed all comments

[Copilot]

Pull request overview

This PR aims to fix observer lifecycle discrepancies when CommandExecutor is wrapped with WithObservers (issue #2440), and adds a regression test.

Changes:

• Update WithObservers::run_target to invoke pre_exec_all/post_exec_all on its own observer tuple.
• Refactor CommandExecutor execution flow by introducing execute_command_only and routing execute_input_with_command through it.
• Add a new unit test intended to verify observer callbacks with WithObservers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
crates/libafl/src/executors/with_observers.rs	Adds explicit observer pre/post calls inside WithObservers::run_target.
crates/libafl/src/executors/command.rs	Splits execution into a “command-only” helper, adjusts observer calls, and adds a WithObservers test.

Comments suppressed due to low confidence (1)

crates/libafl/src/executors/command.rs:396

• The doc comment for execute_command_only says to use it when wrapping with WithObservers and to “let the fuzzer manage the observer lifecycle”, but the function currently calls pre_exec_all itself. This is inconsistent with the comment and makes it easy to accidentally introduce double lifecycle calls. Either update the comment to match the actual behavior, or (preferably) remove observer calls from execute_command_only so the comment is accurate.

    /// Use this when wrapping with `WithObservers` - build `CommandExecutor` with
    /// empty observers `()`, wrap with `WithObservers`, and let the fuzzer manage
    /// the observer lifecycle.
    fn execute_command_only<TB: ToTargetBytesConverter<I, S>>(
        &mut self,
        target_bytes_converter: &mut TB,
        state: &mut S,
        input: &I,
    ) -> Result<ExitKind, Error> {
        use wait_timeout::ChildExt;

        self.observers_mut().pre_exec_all(state, input)?;
        *state.executions_mut() += 1;
        let mut child = self

[Copilot]

WithObservers::run_target now calls pre_exec_all/post_exec_all on its observer tuple. StdFuzzer::execute_input already calls these for any Executor + HasObservers (see crates/libafl/src/fuzzer/mod.rs:1328-1338), so wrapping an executor with WithObservers will invoke observer lifecycle twice during normal fuzzing. This can skew observer state/metrics and adds overhead. Consider reverting WithObservers::run_target back to delegating only, and address the CommandExecutor/WithObservers mismatch in CommandExecutor (or via a CommandExecutor-specific wrapper) without duplicating the global observer lifecycle.

Suggested change

	self.observers.pre_exec_all(state, input)?;
	let exit_kind = self.executor.run_target(fuzzer, state, mgr, input)?;
	self.observers.post_exec_all(state, input, &exit_kind)?;
	Ok(exit_kind)
	self.executor.run_target(fuzzer, state, mgr, input)

[Copilot]

execute_input_with_command calls self.observers_mut().pre_exec_all(...) and then calls execute_command_only, which currently also calls pre_exec_all(...). This results in a guaranteed double pre_exec for every command execution (even without WithObservers), which can corrupt observer state and adds overhead. Make execute_command_only truly “command only” by removing observer lifecycle calls from it, and keep pre_exec_all in only one place (either the caller or the callee, but not both).

Suggested change

	self.observers_mut().pre_exec_all(state, input)?;
	let exit_kind = self.execute_command_only(target_bytes_converter, state, input)?;
	self.observers_mut()
	.post_exec_child_all(state, input, &exit_kind)?;
	Ok(exit_kind)
	self.execute_command_only(target_bytes_converter, state, input)

[Copilot]

In test_with_observers, the inner block re-imports crate::executors::HasObservers even though it is already imported at the top of the function (use crate::executors::{HasObservers, WithObservers}), making the inner use redundant. With #![deny(unused)] enabled for tests, this redundant import is likely to trigger an unused_imports error. Removing the inner use crate::executors::HasObservers; should avoid the lint failure.

Suggested change

use crate::executors::HasObservers;

[Copilot]

The assertion that at least one runtime has as_micros() > 0 can be flaky on fast systems/VMs where the echo process may complete within the same microsecond, yielding a Duration that rounds down to 0µs. Since the test already asserts both observers have last_runtime().is_some(), consider dropping the non-zero micros assertion or using a weaker check (e.g., as_nanos() > 0 or simply validating both are Some).

Suggested change

	// Both observers should have recorded similar runtimes (same execution)
	let runtime1 = time_obs1.last_runtime().unwrap();
	let runtime2 = time_obs2.last_runtime().unwrap();
	assert!(
	runtime1.as_micros() > 0 || runtime2.as_micros() > 0,
	"At least one observer should have recorded non-zero runtime"
	);