fix/1468-impl
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
f829e68911 |
fix(actor): resolve registry.add() rejection of spec-compliant actor YAML
CI / benchmark-regression (push) Has been skipped
CI / helm (push) Successful in 29s
CI / build (push) Successful in 50s
CI / lint (push) Successful in 1m3s
CI / typecheck (push) Successful in 1m23s
CI / quality (push) Successful in 1m38s
CI / security (push) Successful in 1m43s
CI / push-validation (push) Successful in 27s
CI / integration_tests (push) Successful in 3m36s
CI / e2e_tests (push) Successful in 4m6s
CI / unit_tests (push) Successful in 4m53s
CI / docker (push) Successful in 1m29s
CI / coverage (push) Successful in 11m4s
CI / status-check (push) Successful in 4s
CI / benchmark-publish (push) Successful in 1h23m24s
CI / lint (pull_request) Successful in 1m10s
CI / typecheck (pull_request) Successful in 1m16s
CI / security (pull_request) Successful in 1m15s
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 36s
CI / helm (pull_request) Successful in 39s
CI / build (pull_request) Successful in 51s
CI / quality (pull_request) Successful in 1m37s
CI / integration_tests (pull_request) Successful in 3m57s
CI / e2e_tests (pull_request) Successful in 4m13s
CI / unit_tests (pull_request) Successful in 4m27s
CI / docker (pull_request) Successful in 1m30s
CI / coverage (pull_request) Successful in 10m39s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Successful in 1h4m37s
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
|
||
|
|
449c33b752 |
style(tests): apply ruff format to run_behave_parallel.py
CI / lint (pull_request) Successful in 19s
CI / quality (pull_request) Successful in 20s
CI / security (pull_request) Successful in 43s
CI / typecheck (pull_request) Successful in 53s
CI / build (pull_request) Successful in 24s
CI / helm (pull_request) Successful in 23s
CI / push-validation (pull_request) Successful in 20s
CI / e2e_tests (pull_request) Successful in 3m43s
CI / unit_tests (pull_request) Successful in 7m32s
CI / docker (pull_request) Successful in 13s
CI / integration_tests (pull_request) Successful in 7m35s
CI / coverage (pull_request) Successful in 13m33s
CI / benchmark-regression (push) Failing after 0s
CI / benchmark-publish (push) Failing after 0s
CI / status-check (pull_request) Successful in 1s
CI / lint (push) Successful in 27s
CI / typecheck (push) Successful in 1m19s
CI / quality (push) Successful in 29s
CI / security (push) Successful in 1m25s
CI / build (push) Successful in 24s
CI / push-validation (push) Successful in 20s
CI / helm (push) Successful in 30s
CI / e2e_tests (push) Successful in 3m35s
CI / unit_tests (push) Successful in 6m6s
CI / integration_tests (push) Successful in 6m18s
CI / docker (push) Successful in 26s
CI / coverage (push) Successful in 12m18s
CI / status-check (push) Successful in 2s
Fix formatting of conditional expression in _is_btrfs_or_overlayfs() that was flagged by ruff format --check in CI. |
||
|
|
4fc4d7863c |
fix(tests): prevent behave-parallel multiprocessing deadlock on btrfs/overlayfs
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 |
||
|
|
b8732dfc6f |
fix(tests): resolve nox unit_tests timeout for agent_skills_loader and skill_search features
CI / build (pull_request) Successful in 17s
CI / helm (pull_request) Successful in 17s
CI / push-validation (pull_request) Successful in 10s
CI / lint (pull_request) Successful in 39s
CI / quality (pull_request) Successful in 50s
CI / typecheck (pull_request) Successful in 52s
CI / security (pull_request) Successful in 53s
CI / e2e_tests (pull_request) Successful in 2m13s
CI / coverage (pull_request) Successful in 5m35s
CI / integration_tests (pull_request) Successful in 6m40s
CI / unit_tests (pull_request) Successful in 7m38s
CI / docker (pull_request) Successful in 1m45s
CI / status-check (pull_request) Successful in 1s
CI / lint (push) Successful in 16s
CI / quality (push) Successful in 17s
CI / build (push) Successful in 23s
CI / helm (push) Successful in 24s
CI / typecheck (push) Successful in 53s
CI / security (push) Successful in 53s
CI / push-validation (push) Successful in 38s
CI / e2e_tests (push) Successful in 3m14s
CI / unit_tests (push) Successful in 6m37s
CI / integration_tests (push) Successful in 6m39s
CI / docker (push) Successful in 12s
CI / coverage (push) Successful in 10m53s
CI / status-check (push) Successful in 1s
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 |
||
|
|
8b2e0c81c5 |
fix(testing): print behave-parallel worker logs only for failed chunks
CI / lint (pull_request) Successful in 20s
CI / helm (pull_request) Successful in 33s
CI / push-validation (pull_request) Successful in 21s
CI / quality (pull_request) Successful in 3m36s
CI / build (pull_request) Successful in 3m44s
CI / typecheck (pull_request) Successful in 4m30s
CI / security (pull_request) Successful in 4m37s
CI / e2e_tests (pull_request) Successful in 6m54s
CI / unit_tests (pull_request) Successful in 9m46s
CI / integration_tests (pull_request) Successful in 9m51s
CI / docker (pull_request) Successful in 1m33s
CI / coverage (pull_request) Successful in 10m53s
CI / status-check (pull_request) Successful in 0s
CI / security (push) Successful in 41s
CI / helm (push) Successful in 31s
CI / push-validation (push) Successful in 33s
CI / lint (push) Successful in 3m17s
CI / build (push) Successful in 3m16s
CI / quality (push) Successful in 3m38s
CI / typecheck (push) Successful in 4m16s
CI / e2e_tests (push) Successful in 6m35s
CI / unit_tests (push) Successful in 10m18s
CI / integration_tests (push) Successful in 10m21s
CI / docker (push) Successful in 1m36s
CI / coverage (push) Successful in 10m47s
CI / status-check (push) Successful in 0s
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 |
||
|
|
37586882b3 |
chore(ci): extract behave-parallel runner script from noxfile.py into scripts/
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Failing after 17s
CI / build (pull_request) Successful in 21s
CI / helm (pull_request) Successful in 23s
CI / security (pull_request) Failing after 46s
CI / typecheck (pull_request) Failing after 49s
CI / coverage (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Has been skipped
CI / unit_tests (pull_request) Failing after 1m48s
CI / docker (pull_request) Has been skipped
CI / quality (pull_request) Successful in 3m41s
CI / e2e_tests (pull_request) Failing after 16m17s
CI / integration_tests (pull_request) Failing after 21m20s
CI / status-check (pull_request) Failing after 1s
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 |