Documents the fix for sqlite3.IntegrityError when agents plan use is called on an action that already has arguments registered via action create.
ISSUES CLOSED: #6856
- Fix grooming-worker Forgejo permissions (deny → allow) to unblock direct API calls
- Route PR label fetching through forgejo-label-manager subagent
- Replace priority-alignment check with milestone enforcement (every issue must have a milestone)
- Add step 11: address non-code review remarks (labels, description, milestone) during grooming
- Clarify grooming-pool-supervisor stale threshold to explicit 24-hour window
- Refactor pr-merge-pool-supervisor main loop into explicit numbered steps
- Add triage strategy section emphasising parallel review checks and immediate worker dispatch
- Tighten merge criteria: explicit APPROVED state, no unresolved REQUEST_CHANGES on current head
- Dispatch workers for all PR processing, not only rebase operations
- Add rule to batch forgejo_list_pull_reviews calls instead of checking serially
Add a Details entry for HAL 9000 describing the plan lifecycle
concurrency race-condition fix (#7989) — wiring LockService into
execute_plan/apply_plan with unique per-invocation owner identities.
ISSUES CLOSED: #7989
The original implementation used plan_id as the owner_id when acquiring
the advisory lock. Because LockService treats owner_id as the caller
identity and allows re-entrant acquisition for the same owner, concurrent
sessions attempting to lock the same plan would all present the same
owner_id and thus silently renew the lock instead of raising
LockConflictError.
This fix generates a unique UUID for each invocation as the owner_id,
ensuring that concurrent sessions present different owners and thus
trigger LockConflictError when attempting to acquire the same plan lock.
The lock is still acquired before the phase transition and released in
a finally block to ensure cleanup even on error.
ISSUES CLOSED: #8067
LockService was implemented but never integrated into the plan execution
path, leaving execute_plan() and apply_plan() unprotected against
concurrent calls on the same plan_id (race condition, issue #7989).
Changes:
- container.py: add _build_lock_service() factory and register
LockService as a Singleton provider; inject it into
PlanLifecycleService via the DI container.
- plan_lifecycle_service.py: accept optional lock_service parameter in
__init__; in execute_plan() and apply_plan() acquire a plan-level
advisory lock before the critical section and release it in a finally
block so the lock is always freed even when exceptions occur.
When lock_service is None (existing tests without DI wiring) the
behaviour is unchanged — locking is silently skipped for backward
compatibility.
Closes#7989
Ensure fail_fast cancels in-flight futures and reports them as CANCELLED.
Add Behave coverage that reproduces the concurrency regression.
ISSUES CLOSED: #7582
Fixes a bug where the root decision was recorded as 'strategy_choice' instead of
the correct 'prompt_definition' type during the Strategize phase. The decision tree
now correctly records the plan's prompt/description as the root decision, ensuring
proper decision tree structure and downstream decision evaluation.
Changes:
- Modified start_strategize() to record prompt_definition as the root decision
- Updated decision question to 'What is the plan prompt?'
- Set chosen_option to plan.description with fallback to action_name or plan_id
- Added test scenario to verify root decision type is prompt_definition
Closes#9061
HAL 9000 was already listed in the Contributors section. This commit
adds a detail entry in the Details section to explicitly document the
nature of HAL 9000's contributions (automated implementation, bug fixes,
and feature development), satisfying the CONTRIBUTING.md requirement to
update CONTRIBUTORS.md when making a change.
ISSUES CLOSED: #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 falling back to 'manual'.
Users who configured custom automation profiles (e.g. 'semi-auto',
'acme/strict') will now receive an actionable error message listing
available built-in profiles. The resolved profile name is also logged
at debug level for observability.
Added BDD regression tests (8 scenarios) covering:
- Built-in profiles resolve correctly
- Plans with no profile set resolve to 'manual'
- Custom/unknown profile names raise ValidationError
- Error message mentions the unknown profile name
- Error message lists available built-in profiles
- Debug log records the resolved profile name
ISSUES CLOSED: #8232
## Summary
Replaces all 234 bare `@skip` occurrences across 82 Behave feature files with the correct TDD issue-capture tagging system described in CONTRIBUTING.md § Bug Fix Workflow.
Previously, the noxfile ran Behave with `--tags=not @skip`, silently excluding all `@skip`-tagged scenarios from every CI run. These tests never ran, never inverted results via the `@tdd_expected_fail` mechanism, and never contributed to coverage — defeating the purpose of TDD issue-capture testing. Every `@skip` occurrence had a commented-out hint line immediately above it showing the intended proper tags (e.g., `# @tdd_issue @tdd_issue_4272 @tdd_expected_fail @skip`), confirming they were all intended for conversion.
## Changes
### Mechanical conversion (234 replacements across 82 files)
- Extracted the proper TDD tags from the comment hint above each `@skip` line, removed `@skip` from the tag set, and replaced the `@skip` line with those tags.
- Removed the now-redundant comment hint lines alongside each replacement.
### Bug-fixed scenarios — `@tdd_expected_fail` removed (84 scenarios)
- After conversion, ran `nox -s unit_tests` to identify which newly-enabled `@tdd_expected_fail` scenarios now **pass** (their referenced bugs have already been fixed). Removed `@tdd_expected_fail` from those 84 scenarios and their corresponding feature-level tags, leaving only the permanent `@tdd_issue @tdd_issue_<N>` regression-guard tags.
- Affected features include: `tdd_tool_runner_env_precedence`, `tdd_automation_profile_session_leak`, `tls_certificate_check`, `project_create_persist`, `resource_type_bootstrap_*`, and 18 others.
### Noxfile cleanup
- Removed all four `--tags=not @skip` arguments from `noxfile.py` (unit_tests and coverage sessions). With zero `@skip` tags remaining in the codebase, this filter was dead code and its presence would mislead future maintainers into thinking `@skip` is still a supported escape mechanism.
### Regression guard files
- Split the regression guards into two focused files:
- `tdd_regression_guards_exec_env.feature` for bug #4281 (exec-env precedence)
- `tdd_regression_guards_session_list.feature` for bug #4271 (session list summary)
- Each file carries only its own `@tdd_issue` tags at the feature level, avoiding cross-contamination via Behave tag inheritance. The `Background` step (`session-list-summary mock`) only appears in the session-list file where it is actually needed.
### Duplicate tag cleanup
- Removed duplicate `@tdd_issue @tdd_issue_4287` tag lines in `tdd_skill_add_regression.feature` (lines 20 and 29).
### Inline comment for retained `@tdd_expected_fail`
- Added inline comment in `ci_workflow_validation.feature:134` explaining why this specific #4227 scenario retains `@tdd_expected_fail` despite #4227 being closed (CI YAML does not encode threshold as a machine-readable value).
### Known edge cases — `@tdd_expected_fail` retained (closed issues, fix on master, scenarios still fail)
The following issues are **closed** and their fixes **are on master**, but the specific test assertions still fail because the fixes address other aspects of the bugs. The `@tdd_expected_fail` tags are functionally correct and must remain until the specific scenario assertions pass:
- `tdd_exec_env_resolution_precedence.feature` — bug #1080 (closed 2026-03-31). The precedence-level-2-vs-4 scenario still fails.
- `session_list_summary_dedup.feature` — bug #3046 (closed 2026-04-05). The dedup-consistency scenarios still fail.
- `actor_add_update_enforcement.feature` — bug #2609 (closed 2026-04-05). The enforcement scenarios still fail.
- `ci_workflow_validation.feature:134` — #4227 (closed 2026-04-08). The CI YAML threshold assertion still fails.
## Verification
- `grep -r "@skip" features/ --include="*.feature"` → **zero results** ✓
- `grep -n "tags=not @skip" noxfile.py` → **zero results** ✓
- `nox -s unit_tests` → **629 features passed, 0 failed** ✓ (up from ~545 before this PR)
- CI all green (coverage ≥ 97%) ✓
- Integration tests (Robot Framework) do not use `@skip` — confirmed no action needed ✓
- E2E tests (Robot Framework) do not use `@skip` — confirmed no action needed ✓
- `CHANGELOG.md` updated with entry for this change ✓
- `CONTRIBUTORS.md` — Rui Hu already listed ✓
## Issues Addressed
Closes#7025
Co-authored-by: CleverThis <hal9000@cleverthis.com>
Reviewed-on: #7221
Reviewed-by: HAL 9000 <HAL9000@cleverthis.com>
Reviewed-by: HAL9001 <hal9001@cleverthis.com>
Co-authored-by: Rui Hu <rui.hu@cleverthis.com>
Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
Renamed backlog-grooming-pool-supervisor to grooming-pool-supervisor and
backlog-grooming-worker to grooming-worker. The grooming agents now analyze
all open issues AND pull requests (not just the backlog). Workers perform a
full 10-point quality analysis on a single item instead of processing
batches. PRs with unaddressed reviews are prioritized. PR labels are synced
from linked issues. Workers post [GROOMED] markers for tracking.
Fixed critical issue: implementation-pool-supervisor now correctly dispatches
workers through tier selectors (tier-haiku/codex/sonnet/opus) instead of
launching implementation-worker directly, ensuring the escalation model
actually sets the correct model tier. Added implementation-worker to all
four tier selector task permissions.
Updated session tags from AUTO-BLOG to AUTO-GROOM. Updated specification
sections 4.2.1, 5.5, 5.5.1, 9.7, and architecture diagram.
ISSUES CLOSED: n/a
Three implementation contracts clarified in response to security/correctness
bugs surfaced by the bug hunt pool:
1. Path containment: Sandbox path validation MUST use Path.is_relative_to()
not string prefix comparison. String prefix allows /tmp/sandboxmalicious
to pass a /tmp/sandbox root check. Canonical implementation provided.
2. Datetime handling: All stored ISO timestamp comparisons MUST parse back
to timezone-aware datetime objects before comparing. String comparison
of ISO timestamps is incorrect when timezone offsets differ in format.
Canonical parse_utc_ts() pattern provided.
3. Plugin protocol validation: Protocol compliance MUST be checked
structurally via issubclass() — never by instantiating the plugin class.
Instantiation runs __init__ side effects before the plugin is approved.
These are minor clarifications (implementation contracts, not architectural
changes) added to the existing Security and Extensibility sections.
Refs: BUG-HUNT issues #7336 (path traversal), #7341 (datetime comparison),
#7331 (plugin instantiation)
- Add docs/modules/context-hydration.md: documents the ACMS context
hydration pipeline (context_tier_hydrator) that fixes the empty
ContextTierService bug (#1028). Covers hydrate_tiers_from_project,
hydrate_tiers_for_plan, file listing strategies, limits, and
fragment metadata.
- Add docs/modules/git-worktree-sandbox.md: documents the
GitWorktreeSandbox class that isolates plan apply changes in a
dedicated git branch/worktree and merges back on commit (#4454).
Covers full lifecycle, error types, branch naming, and fallback
for non-git projects.
- Update docs/architecture.md: add Git Worktree Sandbox Apply section
and extend ACMS section with context hydration note.
- Update mkdocs.yml: add both new module pages to the Modules nav.
ISSUES CLOSED: #6841
Reclassify the ACMS context tier hydration entry as a bug fix and add the sandbox root wiring item so the changelog matches the behavior shipped in PR #4219 while preserving newer [Unreleased] additions.
ISSUES CLOSED: #7628
- Add CHANGELOG.md entry under [Unreleased] ### Fixed for the empty-run
guard added to ApplyValidationSummary.all_required_passed
- Add HAL 9000 to CONTRIBUTORS.md per CONTRIBUTING.md §PR Process Rule 8
Previously the product-builder would adopt existing supervisor sessions
from prior runs and skip launching those already running. This caused
stale sessions to persist with outdated configuration and context.
Now Phase C.0 discovers and gracefully stops all existing supervisor
sessions before Phase C.2 unconditionally launches all 18 fresh.
The pr-reviewer agent was falling back to Forgejo MCP tools for reads
instead of using curl, causing actions to be attributed to the wrong
user account. This change:
- Blocks ALL Forgejo MCP tools (forgejo: '*': deny) instead of only
specific write tools
- Adds explicit curl-only mandate section with clear explanation
- Adds tested curl command reference covering every API operation
the reviewer needs (PR details, diffs, issues, CI status, reviews,
file contents, posting reviews, posting comments, dismissals)
- Adds robots.txt override instruction for our self-hosted instance
- Updates all review process steps to reference curl instead of MCP
The product-builder now reads FORGEJO_REVIEWER_* env vars and passes them
as the sole credentials to pr-review-pool-supervisor, which passes them
through to pr-reviewer workers. No agent in the review chain receives the
primary bot credentials, preventing accidental self-approvals. Simplifies
the reviewer instructions to require formal PR reviews (not just comments)
and removes contradictory references to denied MCP write tools.
CleverAgents Build Agent
HAL9000
clever-agent
hurui200320