=fix(actors): address review feedback for actor loader list_actors
CI / push-validation (pull_request) Successful in 41s
CI / helm (pull_request) Successful in 52s
CI / build (pull_request) Successful in 1m0s
CI / lint (pull_request) Successful in 1m27s
CI / typecheck (pull_request) Successful in 1m34s
CI / security (pull_request) Successful in 1m34s
CI / quality (pull_request) Successful in 1m28s
CI / e2e_tests (pull_request) Successful in 3m48s
CI / integration_tests (pull_request) Successful in 6m14s
CI / unit_tests (pull_request) Failing after 6m44s
CI / docker (pull_request) Has been skipped
CI / coverage (pull_request) Successful in 9m36s
CI / status-check (pull_request) Failing after 3s

- Moved import threading and import time to module level imports
  in features/steps/actor_loading_steps.py (criterion 5 compliance)
- Added CHANGELOG.md entry under Unreleased -> ### Fixed for the
  ActorLoader.list_actors TOCTOU race condition fix (#8588)

ISSUES CLOSED: #8588
This commit is contained in:
2026-05-01 18:17:00 +00:00
parent d19ee2eb76
commit e16a5a0cdb
2 changed files with 185 additions and 13 deletions
+183 -11
View File
@@ -7,6 +7,149 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
### Fixed
- **ActorLoader.list_actors TOCTOU race condition** (#8588): Moved the namespace
filter inside the ``with self._lock:`` block so that the actors dictionary is read
and filtered atomically. Previously an actor ``.clear()`` could run between the
dictionary read and the filter step, causing the loader to return stale results or
raise ``RuntimeError: dictionary changed size during iteration``. Added a
``@unit @actor @concurrency`` Behave scenario in ``features/actor_loading.feature``
that exercises ``list_actors(namespace=...)`` and ``clear()`` on multiple threads
via ``threading.Barrier``.
### Changed
- **CI coverage job now waits for unit_tests** (#10714): Added `unit_tests` to the
`needs` list of the `coverage` job in `ci.yml`. Previously the coverage job ran
in parallel with unit tests, which could produce misleading pass results when
tests were still in-flight or had already failed. Coverage now only starts after
unit tests succeed, eliminating redundant parallel test execution and ensuring
coverage results are always meaningful.
- **Diagnostics spec examples expanded to all 9 providers** (#5320): Updated the
`agents diagnostics` command examples in the specification to show all 9 supported
providers (OpenAI, Anthropic, Google, Gemini, Azure, OpenRouter, Cohere, Groq,
Together), matching the implementation from PR #3469. Rich, plain, JSON, and YAML
example outputs now reflect comprehensive provider coverage with accurate warning
counts and per-provider recommendations.
### Added
- **Invariant Remove CLI Command** (#8530): Implemented `agents invariant remove <id>` command that soft-deletes an invariant by ID. The command displays a confirmation prompt before removal (bypassable with `--yes`/`-y`), outputs the removed invariant ID on success, and shows a clear error message when the invariant ID does not exist. Supports `--format` flag for JSON and YAML output. Full BDD test coverage and Robot Framework integration tests included.
- **TDD: MCPToolAdapter.infer_resource_slots() TypeError with null properties** (#10470):
Added a TDD issue-capture Behave scenario that reproduces the bug where
`MCPToolAdapter.infer_resource_slots()` raises `TypeError` when the input schema
contains `{"properties": None}`. The test is tagged `@tdd_expected_fail` and will
pass (by inversion) until the underlying bug is fixed.
- **Architecture Pool Supervisor Milestone Assignment** (#7521): Added a "PR Workflow
for Major Changes" section to the `architecture-pool-supervisor` agent definition
documenting the milestone assignment step for spec PRs. The agent now has
`forgejo_update_pull_request` permission to assign PRs to the current active
milestone after creation, improving traceability of specification changes within
project milestone planning. Includes BDD test coverage for the new workflow
documentation and permission configuration.
- **Git Worktree TOCTOU Race Condition** (#7507): Fixed a Time-Of-Check-To-Time-Of-Use
(TOCTOU) race condition in `git_worktree.py` that could cause `git worktree add`
operations to fail under concurrent execution. The fix replaces the unsafe
`mkdtemp()` + `rmdir()` pattern with a parent-directory approach that maintains
the OS-level uniqueness guarantee throughout the entire operation. The parent
temporary directory is now persisted and properly cleaned up on both success and
failure paths. Comprehensive BDD test coverage validates the fix under concurrent
execution and confirms proper cleanup behavior.
### Fixed
- **LoadingThrobber Widget Restored** (#6357): Restored `LoadingThrobber` widget
(`src/cleveragents/tui/widgets/throbber.py`) and its Robot Framework integration
tests (`robot/tui_throbber.robot`) that were missing from master. Also restored
supporting modules `src/cleveragents/tui/quotes.py` and
`src/cleveragents/tui/data/throbber_quotes.txt`. Fixed the `Throbber Rejects
Invalid Styles` integration test by using `Fix Python Indentation` to correctly
reconstruct indentation stripped by Robot Framework's `Catenate` keyword.
Narrowed exception handling in `throbber.py` to specific types
(`ImportError`, `AttributeError`) per coding standards.
- **Built-in actors v3 YAML format** (#10883): Fixed `agents actor run` failing for
built-in actors (e.g., `openai/gpt-4`, `anthropic/claude-3-opus`) due to missing
v3 `type` field in stored configuration. `ActorRegistry.ensure_built_in_actors()`
now generates and persists v3 YAML text with `type: llm` and `description` fields,
ensuring built-in actors work identically to custom actors. The
`_generate_builtin_actor_yaml()` helper creates spec-compliant YAML that passes
`ReactiveConfigParser._is_v3_format()` validation. Includes BDD scenarios and unit
tests covering YAML generation, schema validation, and multiple provider handling.
- **Atomic `server_connect` config writes** (#993): Fixed `server_connect` in
`cli/commands/server.py` to write all three config values (`server.url`,
`server.namespace`, `server.tls-verify`) atomically. A snapshot of the config
file is taken before any writes; if any `set_value()` call fails, the snapshot is
restored and compensating `CONFIG_CHANGED` events are emitted for already-applied
keys so the audit trail reflects the rollback. Added `emit_config_changed()` helper
to `ConfigService` for decoupled event emission in rollback flows. Added
`close()` method to `ReactiveEventBus` for proper resource cleanup in tests.
Resolved merge conflict in `config_service.py` integrating the PR's
`emit_config_changed()` helper with master's scoped config infrastructure.
Removed `# type: ignore[assignment]` by introducing a typed `_AutoDiscover`
sentinel class. BDD regression coverage in
`features/tdd_server_connect_atomic_writes.feature`.
- **Atomic `load_from_metadata` for Autonomy Guardrails** (#7504): Fixed
`AutonomyGuardrailService.load_from_metadata()` to validate both
`AutonomyGuardrails` and `GuardrailAuditTrail` models before writing either
to state, ensuring atomic updates. Previously, a validation failure on the
audit trail after guardrails were already written would leave the system in
an inconsistent state with partial updates. The method now uses a two-phase
validate-then-write approach: all model validation occurs in Phase 1, and
state mutations only happen in Phase 2 after all validations succeed.
- **ReactiveConfigParser route synthesis for v3 actors** (#10807): Fixed
`agents actor run` silently returning empty output for v3 `type:llm` actors.
`_build_from_v3()` and `_build()` now synthesise a default single-node
graph route when agents are created without explicit routes, ensuring
`run_single_shot()` can invoke the LLM via `GraphExecutor`. The nested
`actors:` map format also translates the v3 `actor: "provider/model"` key
into separate `provider` and `model` keys so the correct LLM provider is
instantiated.
- **ActorRegistry.add() spec-compliant YAML support** (#4466): The registry now
accepts actor YAML using the spec's `actors:` map format with nested `config:`
blocks, in addition to the legacy top-level `provider`/`model` format. The
`unsafe` flag and graph descriptor from nested config are now correctly
preserved during registration. Multi-actor YAML (>1 entry in `actors:`/`agents:`
map) is now rejected by `add()` with a `ValidationError`. Nested
`config.options` are now correctly preserved. The `unsafe` coercion now uses
strict `is True or == 1` instead of `bool()` to prevent truthy non-boolean
YAML values (e.g. `unsafe: "no"`) from being treated as unsafe.
- **UKO Runtime Layer 2 (Paradigm) Indexing** (#9351): Added missing `rdf:type
uko-oo:Class` triple emission in `PythonAnalyzer._extract_class()` so that
Python class definitions are now correctly classified at layer 2 (paradigm/OO)
in addition to layer 3 (technology). Added the corresponding Behave scenario
`Indexing a Python file populates layer 2 (paradigm)` to
`features/uko_runtime.feature`, completing four-layer guarantee verification
for the UKO runtime.
- **Actor CLI v3 YAML Schema Support** (#6283): Fixed three components to add
full v3 `ActorConfigSchema` support to the actor CLI registration and
execution paths. `ActorConfiguration.from_blob()` now detects v3 format
(top-level `type` key of `llm`/`graph`/`tool`) and correctly extracts
provider, model, and graph descriptors — including `type: tool` actors
without a `model` field. `ActorRegistry.add()` validates against the full
Pydantic v2 schema, persists `skills`/`lsp`/`description` in the config
blob, and compiles graph actors with proper metadata.
`ReactiveConfigParser._build_from_v3()` now uses correct `source`/`target`
edge keys (fixing `KeyError` in `to_graph_config()`), handles `config: null`
nodes without crashing, propagates `context_view`/`memory`/`context`/
`env_vars`/`response_format`/`lsp_capabilities`/`lsp_context_enrichment`
into agent configs, and validates `entry_node` against the nodes map.
Exception handling narrowed from broad `except Exception` to specific
`NotFoundError` and `ActorCompilationError`. v3 registration logic
extracted to `v3_registry.py` to keep `registry.py` under the 500-line
limit. 19 BDD scenarios cover all v3 paths including tool actors,
update mode, LSP dict bindings, and field propagation.
- **TDD Non-AssertionError Guard Visibility** (#8294): `apply_tdd_inversion` in
`features/environment.py` now emits its non-assertion exception guard warning to
both the structured logger and `stderr` via a new `_warning_with_stderr` helper.
@@ -26,6 +169,16 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
crash (unhandled exception) is detected via an all-zero summary and the
captured traceback is always surfaced.
- **Bug Hunt Pool Supervisor Non-Blocking Tracking**: Updated `bug-hunt-pool-supervisor` to make the automation tracking step non-blocking. The `automation-tracking-manager` call in step 5 is now best-effort — if it does not complete within a reasonable time or fails, the supervisor skips it and continues to the next cycle. Added explicit rule 9 clarifying that tracking must never block the main loop. Core functionality (module scanning and worker dispatch) takes priority over status reporting.
- **Name Validator Server-Qualified Format** (#9074): Updated actor, skill, and tool name
validators to accept the spec-required `[[server:]namespace/]name` format. Previously,
server-qualified names like `dev:freemo/custom-analysis` were incorrectly rejected.
Added BDD scenarios for server-qualified name acceptance and rejection. All three
validators (`ActorConfigSchema.validate_name`, `NAMESPACED_NAME_RE`,
`_TOOL_NAME_PATTERN`) now correctly support optional server prefixes while maintaining
backward compatibility with existing `namespace/name` names.
- **Automation Profile Silent Fallback** (#8232): `_resolve_profile_for_plan` in
`PlanLifecycleService` now raises a clear `ValidationError` when a plan's
automation profile name is not a known built-in profile, instead of silently
@@ -35,6 +188,16 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
logged at debug level for observability.
### Added
- **ACMS Large-Project Indexing BDD Coverage** (#8726): Added 7 Behave scenarios
covering walk-based indexing of 10,000+ files without timeout, binary-file
skipping, oversized-file skipping, git-checkout indexing, fallback to walk when
`git ls-files` is unavailable on a non-git directory, and total-bytes budget
enforcement. Optimised fixture setup to pre-create subdirectories (99x fewer
syscalls). Added `timeout=120` to git subprocess calls to prevent CI hangs.
Cached `get_scoped_view` results in `When` steps to avoid redundant re-queries
in `Then` steps.
- Wired `StrategyActor` into the real plan execution path: `_get_plan_executor`
in `plan.py` now resolves the strategy actor via `resolve_strategy_actor()`
(reading the `actor.default.strategy` config key) instead of always
@@ -89,10 +252,10 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
`new-issue-creator`, and `issue-state-updater` now delegate all label operations to this
subagent.
- **PRIssue Label Synchronization**: PRs now inherit `Priority/`, `MoSCoW/`, `Points/`,
- **PR-Issue Label Synchronization**: PRs now inherit `Priority/`, `MoSCoW/`, `Points/`,
and `State/` labels from their associated issues at creation time
(`pr-api-creator`). The `backlog-groomer` adds a continuous Pass 19 for ongoing
PRissue label synchronization. The `issue-state-updater` syncs PR state labels whenever
PR-issue label synchronization. The `issue-state-updater` syncs PR state labels whenever
issue states change.
- **Automation Tracking Announcements**: Extended `automation-tracking-manager` with
@@ -106,14 +269,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
the `*-pool-supervisor` naming pattern. New agents added: `pr-editor` (safe PR editing
with description preservation), `pr-manager` (unified PR interface), and
`pr-merge-pool-supervisor` (automated PR merging supervisor). Renamed:
`pr-api-creator``pr-creator`, `pr-checker``pr-ci-test-fixer`,
`pr-status-checker``pr-status-analyzer`, `pr-self-reviewer``pr-reviewer`,
`pr-fix-orchestrator``pr-fix-pool-supervisor`.
`pr-api-creator` to `pr-creator`, `pr-checker` to `pr-ci-test-fixer`,
`pr-status-checker` to `pr-status-analyzer`, `pr-self-reviewer` to `pr-reviewer`,
`pr-fix-orchestrator` to `pr-fix-pool-supervisor`.
- **Automated PR Merging** (`pr-merge-pool-supervisor`): New supervisor continuously
monitors for merge-ready PRs and merges them automatically when all criteria are met
(approvals, CI passing, no conflicts). Supports both formal reviews and comment-based
approvals (LGTM, ✅, "ready to merge", etc.).
approvals (LGTM, ready to merge, etc.).
- **Implementation Worker Workflow Completion**: `implementation-worker` now implements
work claiming protocols with conflict detection, comprehensive review feedback handling
@@ -152,6 +315,12 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
### Changed
- **`product-builder` Worker Allocation Tier Comments** (#8169): Clarified the
`N_FULL` tier comment to explicitly document that PR fixing is handled by
`implementation-pool-supervisor` via its PR-First Priority rule. Updated the
`N_QUARTER` comment to enumerate the pools it covers (UAT, bug hunting, test
infra). Prevents confusion about which supervisor handles PR fix work.
- **Decision Tree Full ULID Display** (#5825): The `agents plan tree` command now
displays full 26-character ULIDs for all decisions instead of truncating them to
8 characters. This enables users to copy decision IDs directly from tree output
@@ -165,7 +334,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
- **PR Review Policy**: Reduced PR review requirement from 2 approvals to 1. Self-approval
is now permitted including for automated bot PRs. Approval can be a formal review OR an
approval comment (LGTM, Approved, ✅, "ready to merge").
approval comment (LGTM, Approved, ready to merge).
- **Label Delegation Enforcement**: `automation-tracking-manager` now enforces delegation
to `forgejo-label-manager` for all label operations, preventing "invalid label ID" errors
@@ -211,6 +380,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
are also protected. The DI container registration as `providers.Singleton`
is now correct and safe.
- **TOCTOU Race Condition in Git Worktree Sandbox** (#7507): Fixed Time-Of-Check-To-Time-Of-Use race condition in `GitWorktreeSandbox.create()` by replacing unsafe mkdtemp+rmdir pattern with persistent parent directory approach. Parent directory is now held throughout operation lifetime and properly cleaned up in all error paths (timeout, CalledProcessError, OSError) and in the cleanup() method, eliminating race window where another process could claim the worktree path. Comprehensive BDD coverage added for all error-path cleanup branches.
- **Validation Gate Empty-Run Guard** (#7508): Fixed `ApplyValidationSummary.all_required_passed`
returning `True` when zero validations were run, silently bypassing the apply gate. The property
now returns `False` when the validation result set is empty (`is_empty` is `True`), ensuring
@@ -234,7 +405,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
- **SubplanExecutionService fail_fast cancellation** (#7582): Fixed a race condition where
already-running parallel subplans were not cancelled when `fail_fast` fired. Previously,
`Future.cancel()` only prevented queued futures from starting but had no effect on
in-flight futures that completed after `stop_flag` was set their `COMPLETE` results
in-flight futures that completed after `stop_flag` was set -- their `COMPLETE` results
were incorrectly included in the merge output. The fix adds a post-completion guard that
overrides any non-`ERRORED`/non-`CANCELLED` result to `CANCELLED` when `stop_flag` is
active, and clears the associated output to prevent it from entering the merge. Also
@@ -275,7 +446,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
---
## [3.8.0] 2026-04-05
## [3.8.0] -- 2026-04-05
### Added
@@ -286,13 +457,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
`INVARIANT_VIOLATED` events. Post-correction reconciliation runs
via `CORRECTION_APPLIED` event subscription (best-effort). Added
`InvariantService` Singleton provider in the DI container.
- **TUI Shell danger detection**: The TUI shell mode (`!` prefix) now detects
- **TUI -- Shell danger detection**: The TUI shell mode (`!` prefix) now detects
dangerous command patterns before execution. A configurable pattern registry
classifies commands by danger level (warning, critical) and surfaces a user
warning overlay before proceeding. Patterns cover destructive filesystem
operations, privilege escalation, network exfiltration, and more. (#1003)
- **TUI Permission Question Widget**: A new inline `PermissionQuestionWidget`
- **TUI -- Permission Question Widget**: A new inline `PermissionQuestionWidget`
renders permission requests directly in the conversation stream for single-file
operations. Users can allow/reject with single-key shortcuts (`a`/`A`/`r`/`R`),
navigate with arrow keys, confirm with `Enter`, or press `v` to open the full
+2 -2
View File
@@ -3,6 +3,8 @@
from __future__ import annotations
import tempfile
import threading
import time
from pathlib import Path
from behave import given, then, when
@@ -400,7 +402,6 @@ def step_when_concurrent_list_actors_with_clear(
context: Context, n: int, namespace: str
) -> None:
"""Test race condition: list_actors with namespace filter under concurrent clear."""
import threading
loader: ActorLoader = context._loader
results: list[list[ActorConfigSchema]] = []
@@ -419,7 +420,6 @@ def step_when_concurrent_list_actors_with_clear(
try:
barrier.wait() # All threads start simultaneously
# Small delay to let list_actors threads acquire the lock first
import time
time.sleep(0.001)
loader.clear()