Fix the integration test helper so that it can load scripts/run_behave_parallel.py
outside of nox sessions. The helper imports the runner module via importlib, which
triggers the top-level from behave_pass_suppress_formatter import
PassSuppressFormatter. When invoked from integration tests (or any non-nox
context), neither behave_pass_suppress_formatter nor the behave_parallel
package (created by noxfile.py for unit_tests) is on sys.path, causing a
ModuleNotFoundError and all 6 Robot tests to fail with exit code 1.
The fix adds scripts/ to sys.path before loading the runner module so that
from behave_pass_suppress_formatter import PassSuppressFormatter resolves
correctly. This mirrors the approach already used in noxfile.py's unit_tests
session for the behave-parallel package.
Also addresses review feedback:
- Removes one blank line from scripts/run_behave_parallel.py to bring it to
499 lines, satisfying the project's <500-line code style rule.
- Moves in-function behave imports (Configuration, StreamOpener) in
features/steps/behave_parallel_log_filtering_steps.py to the top-level
import block alongside the existing behave imports, complying with the
project's rule that all imports must be at the top of the file.
Refs: #10987
Implemented PassSuppressFormatter, a custom Behave formatter that buffers
per-scenario output and only flushes it to stdout when the scenario failed
or errored. Passing scenarios produce no output, keeping an all-passing
suite at ~5-10 lines (the _print_overall_summary block only).
Key design decisions:
- PassSuppressFormatter (in scripts/behave_pass_suppress_formatter.py)
inherits from behave's Formatter base class so it can be registered via
behave.formatter._registry.register_as(), which enforces issubclass(cls,
Formatter).
- _SUPPRESS_STATUSES = {'passed', 'skipped'} determines which terminal
statuses are silently discarded; all others (failed, undefined, etc.)
trigger a buffer flush so no failure is hidden.
- _make_runner() in run_behave_parallel.py registers the formatter and
sets it as the default format whenever config.format is None (no
explicit -f/--format flag). Coverage mode (BEHAVE_PARALLEL_COVERAGE=1)
bypasses the formatter and falls back to config.default_format so
slipcover can instrument a single process.
- The formatter lives in a separate file to keep both scripts under the
500-line limit. _install_behave_parallel() in noxfile.py copies both
files into the behave_parallel package. A try/except import handles
both the direct-script path (tests via importlib) and the installed-
package path (nox CI).
- Three new BDD scenarios in behave_parallel_log_filtering.feature cover:
(1) passing scenario -> no output, (2) failing scenario -> full output,
(3) mixed run -> only failing scenario visible. All 20 scenarios pass.
ISSUES CLOSED: #10987
The ActorRegistry.add() method rejected spec-compliant YAML that uses the
actors: map format with nested config: blocks because it only looked for
provider/model at the top level of the blob. Four changes fix this:
1. _extract_v2_actor() now handles both the spec-canonical actors: key and
the legacy agents: key, with actors: taking precedence. It also supports
the combined actor field format (e.g. "openai/gpt-4") from the spec.
2. _extract_v2_options() mirrors the same actors:/agents: support.
3. registry.add() now unconditionally calls _extract_v2_actor() so that
nested unsafe flags and graph descriptors are always captured — even
when top-level provider/model are present. This eliminates the
behavioural asymmetry with from_blob().
4. The unsafe confirmation gate now runs before the duplicate-actor check,
and the graph_descriptor resolution uses explicit is-not-None checks
to distinguish "not set" from "set to empty dict".
Review fixes (cycle 4):
- Added 9 new Behave scenarios: _extract_v2_options edge cases (empty map,
None, list, missing options key), _extract_v2_actor with unsafe=True,
add() with missing name field, top-level unsafe: true (rejection +
acceptance), and multi-actor unsafe limitation documentation.
- Added graph descriptor assertions to all _extract_v2_actor direct
scenarios that were missing them.
- Fixed unsafe field coercion to use explicit boolean check (is True or
== 1) instead of bool() to prevent truthy non-boolean values like
"no" from being treated as unsafe.
- Added legacy graph key fallback (blob.get("graph")) in add() for
consistency with from_blob().
- Fixed _StubActorService.upsert_actor to handle set_default parameter
and pass non-None config_blob to Actor.compute_hash().
- Updated stale CLI comment about registry.add() capabilities.
- Applied ruff format to step definitions.
Review fixes (cycle 5 — external review):
- T1/T2/T3: Added unsafe coercion edge-case tests for 1.0 (float→True),
2 (int>1→False), and 0 (int zero→False).
- T4/T5/T6: Added _extract_v2_options tests for non-dict first entry,
missing config block, and actors: preference over agents:.
- T7: Added integration test for top-level graph_descriptor key.
- T8: Added integration tests for top-level provider_type/model_id aliases.
- T9: Added integration tests for top-level unsafe string coercion ("yes"/"no").
- S1: effective_unsafe now includes unsafe/allow_unsafe params to match
from_blob() and spec rule.
- S2: CHANGELOG entry expanded with multi-actor rejection, options
preservation, and coercion fix.
- B1: Changed v2_options guard from truthiness to is-not-None check.
- P1: _extract_v2_options() now returns a shallow copy.
- P2: Nested options overwrite non-dict top-level options values.
- Q3: Fixed comment variable name (unsafe_raw → top_unsafe_raw).
- S3: Added spec-extension comment for provider_type/model_id aliases.
- S4/B3: Cached actors_raw in multi-actor guard; added cross-ref comment.
- Q5: Added TODO(#10832) to _StubActorService duplicate.
- Q6: Replaced misleading (M2) label with (unsafe coercion).
Review fixes (cycle 6 — agent review):
- MAJ-1: Removed allow_unsafe from effective_unsafe computation.
allow_unsafe is a permission flag, not an assertion; it should not
mark a non-unsafe actor as unsafe. Updated docstring to clarify
the semantic difference between unsafe and allow_unsafe.
- MIN-1: Changed provider/model alias resolution in _extract_v2_actor()
from or-based to is-not-None checks for consistency with
_resolve_actors_map() and add().
- MIN-2: Added test scenario for allow_unsafe=True on non-unsafe YAML.
- MIN-3: Added test scenarios for top-level unsafe: 1 (integer)
rejection without flag and acceptance with flag.
- MIN-4: Added test scenario for graph descriptor additional keys loop
(routes key propagation).
- MIN-5: Added test scenario for non-dict top-level options with nested
options.
- MIN-9: Applied ruff format to step file (removed unnecessary parens).
- NIT-1: Differentiated redundant scenario to test graph descriptor
agent key value instead of duplicating assertions.
- NIT-2: Added test scenario verifying config_blob source: "yaml" default.
- NIT-5: Changed _resolve_actors_map() return type to use
Literal["actors", "agents"] for stronger type safety.
Includes 78 Behave scenarios covering spec-compliant actors: map,
legacy agents: map, top-level fields, rejection of missing provider/model,
combined actor field edge cases, update=True path, schema_version and
compiled_metadata forwarding, actors-as-list edge case, empty actors dict
blocking agents fallback, malformed actor field parts, reverse precedence
for the combined actor field, _extract_v2_options edge cases, unsafe=True
detection, missing name rejection, top-level unsafe, multi-actor
rejection, unsafe coercion edge cases (1.0, 2, 0, "yes", "no"),
top-level graph_descriptor, provider_type/model_id aliases,
_extract_v2_options structural parity with _extract_v2_actor,
allow_unsafe on non-unsafe YAML, top-level unsafe: 1 (integer),
graph descriptor additional keys, non-dict top-level options,
and config_blob source default.
ISSUES CLOSED: #4466
Detect btrfs and overlayfs filesystems and automatically fall back to sequential
mode to prevent deadlocks caused by SQLite WAL file locking and btrfs COW copy-up
locks when multiple forked workers try to access the same files simultaneously.
The fix adds a _is_btrfs_or_overlayfs() function that:
1. Attempts to detect the filesystem type using stat command
2. Falls back to reading /proc/mounts if stat fails
3. Returns True if the filesystem is btrfs or overlayfs
The sequential mode condition is updated to include this check, ensuring that
on affected filesystems, all features run in a single process instead of being
split across multiple forked workers.
ISSUES CLOSED: #9390
Adjusted test running and file-detection logic to stabilize unit tests in overlayfs environments and improve target feature handling.
- Modified scripts/run_behave_parallel.py to run sequentially when there are 2 or fewer feature files, avoiding fork deadlocks on overlayfs and reducing nox-based unit test timeouts for agent_skills_loader and skill_search features.
- Updated noxfile.py to correctly detect feature files in posargs, fixing the prior logic that appended the "features/" directory when specific feature files were provided. This ensures precise test selection and avoids unnecessary path expansion.
Rationale:
These changes address the root causes of flaky unit test timeouts by preventing problematic forking behavior with small feature sets and by ensuring nox respects explicitly provided feature file paths.
ISSUES CLOSED: #9374
In parallel mode, the behave runner previously replayed captured
stdout/stderr for every worker chunk, creating noisy output that
obscured failure diagnostics in CI and local runs.
Changes to scripts/run_behave_parallel.py:
- Added _chunk_has_failures() and _chunk_no_scenarios_ran() helpers
to evaluate individual chunk summaries for failure/error/crash
conditions.
- Updated the aggregation loop in main() to conditionally replay
captured stdout/stderr only for chunks whose summary indicates
failures, errors, or no scenarios ran (crash detection). Passing
chunks now suppress their output entirely.
- Added robust exception handling in _worker_run_features() so that
worker crashes produce a full traceback in stderr and return a crash
summary with features.errors = 1, enabling the parent to detect the
crash via _chunk_has_failures (and also _chunk_no_scenarios_ran,
since no scenarios reached a terminal state) and replay the
diagnostics.
- The conditional replay uses summary-based checks rather than the
raw runner.run() boolean, consistent with the existing exit-code
logic. This avoids spurious log replay for @tdd_expected_fail
scenarios whose runner.run() returns True even though the TDD
inversion handler has corrected the scenario status to passed.
- Existing summary merge, exit semantics, and the no-scenarios
safety net are fully preserved.
New Behave unit tests (17 scenarios) cover the chunk-level helpers,
the conditional aggregation loop, the pure no-scenarios-ran path,
stderr replay for non-crash failed chunks, and the worker crash path.
New Robot integration tests (6 test cases) verify the same behavior
end-to-end via the helper_behave_parallel_log_filtering.py script.
Also updated:
- CHANGELOG.md: add unreleased entry for this behavioral change.
- features/steps/behave_parallel_log_filtering_steps.py: use
contextlib.redirect_stdout/redirect_stderr instead of manual
sys.stdout assignment; register module in sys.modules; document
CWD requirement in _load_runner_module().
- robot/helper_behave_parallel_log_filtering.py: move import io to
top-level; remove redundant inline imports; use contextlib for
output capture; register module in sys.modules; document CWD
requirement.
Branch note: the canonical branch for this fix is
bugfix/m3-behave-parallel-failed-chunk-logs. The PR head branch
(bugfix/mX-behave-parallel-failed-chunk-logs) cannot be renamed via
the Forgejo API; both branches are kept in sync at the same SHA.
ISSUES CLOSED: #8351
Move the large embedded `_BEHAVE_PARALLEL_CLI_SOURCE` string constant out of
noxfile.py and into a standalone `scripts/run_behave_parallel.py` module.
The `_install_behave_parallel()` helper now reads the script from disk via
`Path(__file__).parent / 'scripts' / 'run_behave_parallel.py'` instead of
embedding the source as a raw string literal. This allows ruff to lint and
type-check the runner independently, and makes noxfile.py significantly
shorter and easier to read.
No functional changes: the installed `behave-parallel` entry point is
identical to the previous embedded version. Parallel and sequential modes,
coverage integration, and the multiprocessing fork model are all preserved.
Fixed two SIM105 lint violations in the extracted script (replaced
try/except/pass with contextlib.suppress).
ISSUES CLOSED: #1538