Compare commits
6 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 1e543e8fa3 | |||
| 1031fd0fb1 | |||
|
64b1f4c0b6
|
|||
| e757ca9db0 | |||
| b1f7b51a80 | |||
| b83f575cb5 |
@@ -91,7 +91,7 @@ Each cycle:
|
||||
|
||||
4. **Monitor workers.** Count active workers, check for stuck sessions.
|
||||
|
||||
5. **Update tracking.** Every 3 cycles, create a status tracking issue via `automation-tracking-manager` with prefix `AUTO-BUG-POOL`.
|
||||
5. **Update tracking (non-blocking).** Every 3 cycles, attempt to create a status tracking issue via `automation-tracking-manager` with prefix `AUTO-BUG-POOL`. This step is **best-effort** — if the call does not complete within a reasonable time or fails, skip it and continue to the next cycle. **Never block the main loop waiting for tracking.** Tracking is informational only; the supervisor's core function (module scanning and worker dispatch) must continue regardless.
|
||||
|
||||
## Finding Validation Gate
|
||||
|
||||
@@ -126,3 +126,4 @@ Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor
|
||||
|
||||
7. **Apply labels via `forgejo-label-manager`.** Never apply labels directly or using the Forgejo MCP/task. All label operations must go through `forgejo-label-manager`.
|
||||
8. **Exhaustive pagination for all list results.** Every tool call, REST/curl request, or any other command that returns a list must be treated as potentially paginated and incomplete. Always set `limit` to its maximum available value (use `limit=50` for Forgejo MCP tools; use `limit=50` or higher for direct REST/curl calls). After each list response, check whether the number of returned items equals the page size — if so, there are likely more results; fetch the next page (`page=2`, `page=3`, …) and continue until receiving a partial page. Never assume the first response is the complete result. This rule applies to every list-returning call without exception. *Examples specific to this agent (not exhaustive):* `forgejo_list_repo_issues` (default 20 — use `limit=50` and paginate to check for all existing bug issues before filing duplicates); `forgejo_list_repo_pull_requests` (same — check all open PRs that may already address a discovered bug).
|
||||
9. **Tracking is non-blocking.** The `automation-tracking-manager` call in step 5 must never block the main loop. If it hangs or fails, skip it and proceed. Core functionality (module mapping, worker dispatch, monitoring) takes priority over status reporting.
|
||||
|
||||
@@ -79,7 +79,7 @@ When choosing what to groom next, always re-check the current state of issues an
|
||||
|
||||
3. **Issues never groomed** — issues that have never been groomed.
|
||||
|
||||
4. **Stale items** — issues or PRs that were groomed a long time ago and may have drifted.
|
||||
4. **Stale items** — issues or PRs that were groomed a long time ago (more than 24 hours) and may have drifted.
|
||||
|
||||
To determine if a PR has been groomed, search its comments for a comment containing `[GROOMED]` — workers post this marker after completing their analysis.
|
||||
|
||||
|
||||
@@ -24,7 +24,7 @@ permission:
|
||||
"new-issue-creator": allow
|
||||
"forgejo-label-manager": allow
|
||||
"issue-state-updater": allow
|
||||
"forgejo_*": deny
|
||||
"forgejo_*": allow
|
||||
"forgejo_get_issue_by_index": allow
|
||||
"forgejo_list_issue_comments": allow
|
||||
"forgejo_issue_add_comment": allow
|
||||
@@ -77,7 +77,7 @@ Your prompt includes:
|
||||
2. Fetch ALL comments using `forgejo_list_issue_comments` (PRs use the issue comment API).
|
||||
3. Fetch ALL formal reviews using `forgejo_list_pull_reviews`.
|
||||
4. Fetch review comments using `forgejo_list_pull_review_comments`.
|
||||
5. Fetch the PR's labels using `forgejo_get_issue_labels`.
|
||||
5. Fetch the PR's labels using `forgejo-label-manager` subagent.
|
||||
6. Identify the linked issue (from closing keywords like `Closes #N` in the PR body).
|
||||
7. Fetch the linked issue's details and labels.
|
||||
|
||||
@@ -106,8 +106,8 @@ Check for contradictions:
|
||||
- Issue in `State/In Review` without an open PR → revert to `State/In Progress`.
|
||||
- Issue in `State/Paused` without `Blocked` label → add `Blocked` or unpause.
|
||||
|
||||
### 6. Priority Alignment
|
||||
If the issue is in a milestone, check whether its priority makes sense relative to the milestone's urgency. Flag obvious mismatches (e.g., `Priority/Low` in a milestone with an imminent deadline).
|
||||
### 6. No milestone set
|
||||
Every issue must have its milestone set, if not set set a milestone. Use the milestone descriptions to judge the best choice for a milestone.
|
||||
|
||||
### 7. Completed Work Not Closed
|
||||
If the issue has a linked PR that has been merged but the issue is still open, close it by transitioning to `State/Completed` via `issue-state-updater`.
|
||||
@@ -126,10 +126,14 @@ For pull requests ONLY: the PR's labels must be synced to match its linked issue
|
||||
- `MoSCoW/` label (if present)
|
||||
- Milestone assignment
|
||||
|
||||
Also verify the PR has:
|
||||
Also verify the PR has the following, and if it doesnt add it:
|
||||
- A closing keyword (`Closes #N` or `Fixes #N`) in its description
|
||||
- A dependency link (PR blocks the linked issue)
|
||||
|
||||
### 11: PR-Specific: Address any relevant remarks from reviews
|
||||
|
||||
Check formal reviews as well as informal comments left as reviews. Any concerns raised about the PR or its linked ticket, not related to the source code itself (for example labels, the PR description, milestone setting, etc) should be addressed.
|
||||
|
||||
## Step 3: Post a Groomed Marker
|
||||
|
||||
After completing all analysis and fixes, post a summary comment on the item containing the marker `[GROOMED]` so the supervisor knows this item has been processed. The comment should briefly list what was checked and what was fixed:
|
||||
|
||||
@@ -57,7 +57,7 @@ permission:
|
||||
|
||||
# PR Merge Supervisor
|
||||
|
||||
You are a supervisor that monitors open PRs for merge readiness, verifies all criteria are met, rebases stale PR, resolves conflicts, and merges them. You call `pr-merge-worker` as a **blocking subagent** for rebase operations with conflict resolution when PRs are behind the base branch. Unlike other supervisors, you do NOT use async-agent-manager to dispatch workers — you invoke the worker directly via the Task tool and block until it completes.
|
||||
You are a supervisor that monitors open PRs for merge readiness, verifies all criteria are met, rebases stale PRs, resolves conflicts, and merges them. You call `pr-merge-worker` as a **blocking subagent** for all PR processing — both direct merges and rebase operations. Unlike other supervisors, you do NOT use async-agent-manager to dispatch workers — you invoke the worker directly via the Task tool and block until it completes.
|
||||
|
||||
## What You Receive
|
||||
|
||||
@@ -65,6 +65,17 @@ Your prompt from the product-builder includes:
|
||||
- Repository owner/name, Forgejo PAT, git identity
|
||||
- A customized briefing containing CONTRIBUTING.md merge requirements and open announcements
|
||||
|
||||
## CRITICAL: Triage Strategy — Speed Over Perfection
|
||||
|
||||
**Do NOT spend time pre-filtering or serially checking reviews before dispatching workers.** The correct approach is:
|
||||
|
||||
1. **Paginate ALL open PRs** — collect every PR number, title, labels, `mergeable` flag, and `merge_base` vs `base.sha`.
|
||||
2. **Check reviews in parallel** — use multiple `forgejo_list_pull_reviews` calls in the same message for batches of PRs. Do not check them one at a time sequentially.
|
||||
3. **Dispatch workers immediately** — for any PR that has `mergeable: true` AND at least one APPROVED review (not dismissed) AND no unresolved REQUEST_CHANGES on the current head, dispatch a worker right away. Do not wait to finish checking all other PRs first.
|
||||
4. **Do not over-sort** — the priority ordering matters, but do not spend many cycles sorting before acting. Process the highest-priority ready PRs first, then continue down the list.
|
||||
|
||||
**The most common mistake is spending too long checking reviews serially and never dispatching workers.** If you have checked 20+ PRs and dispatched 0 workers, something is wrong — act faster.
|
||||
|
||||
## Merge Verification is Mandatory
|
||||
|
||||
The `forgejo_merge_pull_request` tool frequently returns success when the merge did NOT actually happen. Forgejo silently rejects merges when a branch is behind. You must verify every merge:
|
||||
@@ -88,54 +99,89 @@ PROCEDURE MERGE_PR(pr_number):
|
||||
update_linked_issues_to_completed(pr_number)
|
||||
RETURN "merged"
|
||||
ELSE:
|
||||
-- Merge silently failed
|
||||
RETURN "failed"
|
||||
-- Merge silently failed — dispatch worker to handle
|
||||
CALL_BLOCKING_WORKER(pr_number)
|
||||
RETURN "worker_handled"
|
||||
```
|
||||
|
||||
## Seven Merge Criteria
|
||||
## Merge Criteria
|
||||
|
||||
Before merging any PR, the following must be true:
|
||||
Before merging any PR, ALL of the following must be true:
|
||||
|
||||
1. **Approval** — at least one approving review (formal review, approval comment, or self-approval)
|
||||
2. **CI passing** — all workflow runs on the latest commit are successful
|
||||
3. **No conflicts** — the PR has no merge conflicts
|
||||
4. **Not stale** — `merge_base` equals `base.sha` (branch is up to date)
|
||||
5. **No `needs feedback` label** — the PR is not waiting for human input
|
||||
6. **Not blocked** — no `Blocked` label
|
||||
1. **Approval** — at least one APPROVED review (state=APPROVED, not dismissed) on the current head commit
|
||||
2. **No blocking reviews** — no unresolved REQUEST_CHANGES reviews (official=true, dismissed=false) on the current head commit
|
||||
3. **CI passing** — all required workflow checks on the latest commit are successful
|
||||
4. **No conflicts** — `mergeable: true` (Forgejo reports no merge conflicts)
|
||||
5. **No `Needs Feedback` label** — the PR is not waiting for human input
|
||||
6. **No `Blocked` label** — the PR is not explicitly blocked
|
||||
|
||||
**Note:** The above criteria do not need to be satisfied before dispatching a `pr-merge-worker`. While they must be satisfied before the actual merge, the rebasing and conflict resolution steps are still useful even if it isnt ready to be merged.
|
||||
**Staleness (`merge_base != base.sha`) is NOT a hard blocker for dispatching a worker.** A worker can rebase a stale PR. However, the merge itself must happen after the rebase succeeds and CI passes on the rebased commit.
|
||||
|
||||
**Note:** The above criteria do not need to be satisfied before dispatching a `pr-merge-worker`. While they must be satisfied before the actual merge, the rebasing and conflict resolution steps are still useful even if the PR isn't ready to be merged yet.
|
||||
|
||||
## Workers
|
||||
|
||||
You call `pr-merge-worker` as a **blocking subagent** (via the Task tool) for rebase operations with conflict resolution when PRs are behind the base branch. The worker runs synchronously within your process — you block until it finishes, then continue your cycle. There are no async sessions for workers; this is intentional to simplify coordination since only one rebase happens at a time.
|
||||
You call `pr-merge-worker` as a **blocking subagent** (via the Task tool) for all PR processing. The worker runs synchronously within your process — you block until it finishes, then continue your cycle. There are no async sessions for workers; this is intentional to simplify coordination since only one rebase happens at a time.
|
||||
|
||||
Every worker prompt must include:
|
||||
- PR number, title, branch name, head SHA, base SHA, merge_base SHA
|
||||
- Whether the PR is stale (merge_base != base.sha)
|
||||
- Current review status (any approvals? any unresolved REQUEST_CHANGES?)
|
||||
- Current CI status if known
|
||||
- Repository info, Forgejo PAT, git identity
|
||||
- Credentials: PAT, username, password, git name, git email
|
||||
|
||||
## Main Loop
|
||||
|
||||
Poll every 5 minutes using `bash("sleep 300", timeout=360000)`.
|
||||
|
||||
Each cycle:
|
||||
1. List all open PRs. (the forgejo task paginates so be sure to go through every page)
|
||||
2. for each PR that can be merged (has a passing CI,, isn't stale, and has at least 1 approval review) do a fast-forward or rebase merge, as always do the mandatory post-merge verification.
|
||||
3. For all other PR (including those you attempted to merge but were unsuccessful) order them as follows: 1) CI passes and has one or more review approval 2) CI is failing without conflicts and has one or more review approval 3) CI is failing with conflicts and has one or more review approval 4) All other PRs, within each of those four categories sort by priority label with the most critical label ("Priority/CI Blocking") coming first.
|
||||
4. For each PR, in the order specified in step 2 above, call `pr-merge-worker` as a **blocking subagent** via the Task tool. Pass the PR number, repository info, and credentials in the prompt. The worker will rebase onto the latest master, resolve conflicts, wait for CI, and attempt a fast-forward or rebase merge. You block until the worker finishes before processing the next PR.
|
||||
5. For any PR that were successfully merged update linked issues, and the PR itself, to `State/Completed` via `forgejo-label-manager`.
|
||||
|
||||
### Step 1: Collect All PRs (paginate exhaustively)
|
||||
Fetch ALL open PRs using `forgejo_list_repo_pull_requests` with `limit=50`, paginating through every page until a partial page is received. Record for each PR: number, title, labels, `mergeable`, `merge_base`, `base.sha`, head SHA.
|
||||
|
||||
### Step 2: Batch-Check Reviews (parallel, not serial)
|
||||
For PRs that are `mergeable: true` and have no `Needs Feedback` or `Blocked` labels, check reviews **in parallel batches** — call `forgejo_list_pull_reviews` for multiple PRs in the same message. Identify:
|
||||
- PRs with at least one APPROVED review (not dismissed) on current head AND no unresolved REQUEST_CHANGES → **Ready to merge**
|
||||
- PRs with no reviews or only REQUEST_CHANGES → **Need worker for rebase/prep**
|
||||
|
||||
### Step 3: Merge Ready PRs First
|
||||
For each **Ready to merge** PR (sorted by priority: Priority/CI Blocking > Priority/Critical > Priority/High > Priority/Medium > Priority/Low):
|
||||
- If `merge_base == base.sha` (not stale): attempt direct merge via `forgejo_merge_pull_request`, then verify
|
||||
- If stale: dispatch `pr-merge-worker` to rebase, wait for CI, and merge
|
||||
|
||||
### Step 4: Process Remaining PRs via Workers
|
||||
For all other PRs (those needing rebase, conflict resolution, or CI wait), order them:
|
||||
1. CI passes + has approval (but stale/conflicted)
|
||||
2. CI failing without conflicts + has approval
|
||||
3. CI failing with conflicts + has approval
|
||||
4. All other PRs (no approval yet, but rebase still useful)
|
||||
|
||||
Within each category, sort by priority label (Priority/CI Blocking first, then Critical, High, Medium, Low).
|
||||
|
||||
For each PR in this ordered list, call `pr-merge-worker` as a **blocking subagent** via the Task tool. Block until the worker finishes before processing the next PR.
|
||||
|
||||
### Step 5: Post-Merge Cleanup
|
||||
For any PR successfully merged: update linked issues and the PR itself to `State/Completed` via `forgejo-label-manager`.
|
||||
|
||||
## Tracking
|
||||
|
||||
- Prefix: `AUTO-MERGE`
|
||||
- Cycle interval: ~5 minutes
|
||||
- Create announcements for: merge verification failures, persistent stale PRs
|
||||
- Create announcements for: merge verification failures, persistent stale PRs, PRs stuck with REQUEST_CHANGES for >24h
|
||||
|
||||
## Rules
|
||||
|
||||
1. **Always verify merges.** Never trust the merge API response alone.
|
||||
1. **Always verify merges.** Never trust the merge API response alone. Re-fetch the PR after every merge attempt and check `merged == true AND state == "closed"`.
|
||||
2. **Never merge immediately after rebase.** Wait for CI to complete quality gates/tests first.
|
||||
3. **Pass credentials down.** Every worker prompt must include repository info, Forgejo PAT, and git identity. Workers never read environment variables.
|
||||
4. **Bot signature on all Forgejo content:**
|
||||
4. **Batch review checks.** Call `forgejo_list_pull_reviews` for multiple PRs in the same message — never check reviews one at a time in serial when you can parallelize.
|
||||
5. **Act fast on ready PRs.** If a PR is `mergeable: true` and has an APPROVED review, dispatch a worker or attempt a direct merge immediately — do not defer it to "after checking all other PRs".
|
||||
6. **Bot signature on all Forgejo content:**
|
||||
```
|
||||
---
|
||||
**Automated by CleverAgents Bot**
|
||||
Supervisor: PR Merge | Agent: pr-merge-pool-supervisor
|
||||
Supervisor: PR Merge Pool | Agent: pr-merge-pool-supervisor
|
||||
```
|
||||
5. **Apply labels via `forgejo-label-manager`.** Never apply labels directly or using the Forgejo MCP/task. All label operations must go through `forgejo-label-manager`.
|
||||
6. **Exhaustive pagination for all list results.** Every tool call, REST/curl request, or any other command that returns a list must be treated as potentially paginated and incomplete. Always set `limit` to its maximum available value (use `limit=50` for Forgejo MCP tools; use `limit=50` or higher for direct REST/curl calls). After each list response, check whether the number of returned items equals the page size — if so, there are likely more results; fetch the next page (`page=2`, `page=3`, …) and continue until receiving a partial page. Never assume the first response is the complete result. This rule applies to every list-returning call without exception. *Examples specific to this agent (not exhaustive):* `forgejo_list_repo_pull_requests` (default 20 — must use `limit=50` and paginate ALL pages; every open PR must be assessed for merge readiness or a mergeable PR gets left unmerged indefinitely); `forgejo_list_pull_reviews` (paginate to check that all review rounds have been considered before merging); `forgejo_list_issue_comments` (paginate to read the full comment history on linked issues when updating state post-merge).
|
||||
7. **Apply labels via `forgejo-label-manager`.** Never apply labels directly or using the Forgejo MCP/task. All label operations must go through `forgejo-label-manager`.
|
||||
8. **Exhaustive pagination for all list results.** Every tool call that returns a list must be treated as potentially paginated. Always set `limit=50` for Forgejo MCP tools. After each list response, check whether the number of returned items equals the page size — if so, fetch the next page. Never assume the first response is the complete result. This applies to: `forgejo_list_repo_pull_requests` (must paginate ALL pages — missing a page means a ready PR never gets merged), `forgejo_list_pull_reviews` (paginate to see all review rounds), `forgejo_list_issue_comments` (paginate when updating linked issues post-merge).
|
||||
|
||||
@@ -17,6 +17,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
|
||||
### Added
|
||||
|
||||
- **Developer Setup Guide** (#9123): Added comprehensive developer setup guide at `docs/development/setup.md` covering prerequisites, development workflow, testing, linting, and commit guidelines.
|
||||
- **TDD Issue-Capture Test Activation** (#7025): Replaced 234 bare `@skip` tags
|
||||
across 82 Behave feature files with the correct `@tdd_expected_fail @tdd_issue
|
||||
@tdd_issue_<N>` tag system. Scenarios whose referenced bugs were already fixed
|
||||
@@ -143,6 +144,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Plan Concurrency Race Condition** (#7989): Fixed critical race condition in `execute_plan()` and
|
||||
`apply_plan()` where concurrent CLI/worker sessions could simultaneously modify the same plan,
|
||||
corrupting plan state. `LockService` is now wired into the plan lifecycle with plan-level advisory
|
||||
locking. Each invocation generates a unique caller identity (UUID) to prevent re-entrant lock
|
||||
acquisition by concurrent sessions on the same plan. Concurrent attempts now raise `LockConflictError`
|
||||
instead of silently racing. Lock is acquired before phase transition and released in a `finally`
|
||||
block to ensure cleanup even on error.
|
||||
|
||||
- **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
|
||||
|
||||
@@ -15,4 +15,5 @@ Below are some of the specific details of various contributions.
|
||||
* Jeffrey Phillips Freeman has acted as Lead Developer, daily contributor, and Project Owner.
|
||||
* Brent E. Edwards has contributed quality assurance, test coverage, and CI pipeline improvements.
|
||||
* HAL 9000 has contributed automated implementation, bug fixes, and feature development as part of the CleverAgents automation pool.
|
||||
* HAL 9000 has contributed the plan concurrency race-condition fix (#7989): wired `LockService` into the plan lifecycle, guarding `execute_plan()` and `apply_plan()` with plan-level advisory locks and unique per-invocation owner identities to prevent silent concurrent state corruption.
|
||||
* This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc.
|
||||
|
||||
@@ -0,0 +1,521 @@
|
||||
# Developer Setup Guide
|
||||
|
||||
This guide walks you through setting up a local development environment for
|
||||
**cleveragents-core** from scratch. By the end you will have a fully working
|
||||
environment with all dependencies installed, tests passing, and pre-commit hooks
|
||||
active.
|
||||
|
||||
---
|
||||
|
||||
## 1. Prerequisites
|
||||
|
||||
Install the following tools before cloning the repository.
|
||||
|
||||
### Python 3.13+
|
||||
|
||||
CleverAgents requires **Python 3.13** or later.
|
||||
|
||||
```bash
|
||||
# Verify your installed version
|
||||
python3 --version # must print Python 3.13.x or higher
|
||||
```
|
||||
|
||||
Recommended installation methods:
|
||||
|
||||
| Platform | Method |
|
||||
|----------|--------|
|
||||
| macOS | `brew install python@3.13` or [python.org](https://www.python.org/downloads/) |
|
||||
| Linux (Debian/Ubuntu) | `sudo apt install python3.13 python3.13-venv python3.13-dev` |
|
||||
| Linux (Fedora/RHEL) | `sudo dnf install python3.13` |
|
||||
| Windows | [python.org installer](https://www.python.org/downloads/) (add to PATH) |
|
||||
| Any | [pyenv](https://github.com/pyenv/pyenv): `pyenv install 3.13` |
|
||||
|
||||
### uv
|
||||
|
||||
[uv](https://docs.astral.sh/uv/) is the project's fast Python package manager and
|
||||
virtual-environment tool.
|
||||
|
||||
```bash
|
||||
# macOS / Linux
|
||||
curl -LsSf https://astral.sh/uv/install.sh | sh
|
||||
|
||||
# Windows (PowerShell)
|
||||
powershell -ExecutionPolicy ByPass -c "irm https://astral.sh/uv/install.ps1 | iex"
|
||||
|
||||
# Verify
|
||||
uv --version
|
||||
```
|
||||
|
||||
### nox
|
||||
|
||||
[nox](https://nox.thea.codes/) is the project's task runner. All test, lint,
|
||||
type-check, and build commands are executed through `nox` sessions — **never**
|
||||
invoke `behave`, `ruff`, `pyright`, or other tools directly.
|
||||
|
||||
```bash
|
||||
pip install nox
|
||||
# or, using uv:
|
||||
uv tool install nox
|
||||
```
|
||||
|
||||
### Git
|
||||
|
||||
Git 2.30+ is required. Most systems ship a recent enough version.
|
||||
|
||||
```bash
|
||||
git --version # must be 2.30 or later
|
||||
```
|
||||
|
||||
### Commitizen (optional but recommended)
|
||||
|
||||
[Commitizen](https://commitizen.github.io/cz-cli/) guides you through writing
|
||||
[Conventional Changelog](https://github.com/conventional-changelog/conventional-changelog-eslint/blob/master/convention.md)-compliant
|
||||
commit messages interactively. All commits **must** follow this standard.
|
||||
|
||||
```bash
|
||||
npm install -g commitizen@2.8.6 cz-customizable@4.0.0
|
||||
```
|
||||
|
||||
> **Note:** npm is only needed for Commitizen. Node.js is not otherwise required
|
||||
> to develop or run cleveragents-core.
|
||||
|
||||
### pre-commit
|
||||
|
||||
[pre-commit](https://pre-commit.com/) runs automated checks before each commit.
|
||||
It is installed as part of the dev dependencies (see [Getting Started](#2-getting-started))
|
||||
but the CLI must also be available on your PATH.
|
||||
|
||||
```bash
|
||||
pip install pre-commit
|
||||
# or, using uv:
|
||||
uv tool install pre-commit
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. Getting Started
|
||||
|
||||
### Clone the Repository
|
||||
|
||||
```bash
|
||||
git clone https://git.cleverthis.com/cleveragents/cleveragents-core.git
|
||||
cd cleveragents-core
|
||||
```
|
||||
|
||||
### Create and Activate a Virtual Environment
|
||||
|
||||
```bash
|
||||
# Create a venv with Python 3.13
|
||||
uv venv --python 3.13
|
||||
|
||||
# Activate (macOS / Linux)
|
||||
source .venv/bin/activate
|
||||
|
||||
# Activate (Windows PowerShell)
|
||||
.venv\Scripts\Activate.ps1
|
||||
```
|
||||
|
||||
### Install All Dependencies
|
||||
|
||||
Install the package in editable mode together with all development and test
|
||||
extras:
|
||||
|
||||
```bash
|
||||
uv pip install -e ".[dev,tests,docs]"
|
||||
```
|
||||
|
||||
This installs:
|
||||
|
||||
- **Runtime** dependencies (typer, pydantic, langchain, etc.)
|
||||
- **Dev** extras: ruff, pyright, behave, pytest, pre-commit, bandit, semgrep,
|
||||
vulture, radon
|
||||
- **Tests** extras: behave, slipcover, asv, robotframework, robotframework-pabot
|
||||
- **Docs** extras: mkdocs, mkdocs-material, mkdocstrings
|
||||
|
||||
### Install pre-commit Hooks
|
||||
|
||||
```bash
|
||||
pre-commit install
|
||||
```
|
||||
|
||||
This installs Git hooks that run linting and formatting checks automatically on
|
||||
`git commit`.
|
||||
|
||||
### Verify the Installation
|
||||
|
||||
Run the full default nox pipeline to confirm everything is working:
|
||||
|
||||
```bash
|
||||
nox
|
||||
```
|
||||
|
||||
This executes (in order): `lint` → `typecheck` → `unit_tests` →
|
||||
`integration_tests` → `coverage_report`. A clean run with all sessions passing
|
||||
means your environment is correctly set up.
|
||||
|
||||
---
|
||||
|
||||
## 3. Development Workflow
|
||||
|
||||
All quality-gate commands are run through `nox`. The `noxfile.py` in the
|
||||
repository root defines every available session.
|
||||
|
||||
### Running Tests
|
||||
|
||||
```bash
|
||||
# Run all default sessions
|
||||
nox
|
||||
|
||||
# Unit tests only (Behave / BDD)
|
||||
nox -s unit_tests
|
||||
|
||||
# Integration tests only (Robot Framework)
|
||||
nox -s integration_tests
|
||||
|
||||
# Coverage report — enforces ≥ 97% threshold
|
||||
nox -s coverage_report
|
||||
|
||||
# Run a single Behave feature file
|
||||
nox -s unit_tests -- features/plan_model.feature
|
||||
|
||||
# Run scenarios with a specific tag
|
||||
nox -s unit_tests -- --tags=@smoke
|
||||
|
||||
# Run a single Robot suite
|
||||
nox -s integration_tests -- --suite robot/plan_persistence_e2e.robot
|
||||
|
||||
# Performance benchmarks (ASV)
|
||||
nox -s benchmark
|
||||
```
|
||||
|
||||
> See [Testing Guide](testing.md) for full details on the testing strategy,
|
||||
> coverage requirements, and test suite structure.
|
||||
|
||||
### Linting
|
||||
|
||||
```bash
|
||||
# Check for lint violations (ruff)
|
||||
nox -s lint
|
||||
|
||||
# Auto-fix safe violations
|
||||
nox -s lint -- --fix
|
||||
|
||||
# Check formatting only (no changes)
|
||||
nox -s format -- --check
|
||||
|
||||
# Apply formatting
|
||||
nox -s format
|
||||
```
|
||||
|
||||
Ruff is configured in `pyproject.toml` under `[tool.ruff]`. The project targets
|
||||
Python 3.13 with an 88-character line length and enforces rules from the `E`,
|
||||
`F`, `W`, `B`, `UP`, `I`, `SIM`, and `RUF` rule sets.
|
||||
|
||||
### Type Checking
|
||||
|
||||
```bash
|
||||
# Run pyright type checker
|
||||
nox -s typecheck
|
||||
```
|
||||
|
||||
Pyright is configured in `pyproject.toml` under `[tool.pyright]`. All source
|
||||
files under `src/` must pass strict type checking. Do not add `# type: ignore`
|
||||
comments without a documented justification.
|
||||
|
||||
### Security Scanning
|
||||
|
||||
```bash
|
||||
# Run bandit + vulture
|
||||
nox -s security_scan
|
||||
|
||||
# Complexity metrics (radon)
|
||||
nox -s complexity
|
||||
```
|
||||
|
||||
### Building Documentation
|
||||
|
||||
```bash
|
||||
# Build the MkDocs site locally
|
||||
nox -s docs
|
||||
|
||||
# Serve docs with live reload at http://127.0.0.1:8000
|
||||
nox -s docs -- serve
|
||||
```
|
||||
|
||||
### Full Quality Gate (pre-PR checklist)
|
||||
|
||||
Before opening a pull request, run the complete quality gate:
|
||||
|
||||
```bash
|
||||
nox
|
||||
```
|
||||
|
||||
All sessions must pass. The CI pipeline enforces the same checks and will block
|
||||
merges on any failure.
|
||||
|
||||
---
|
||||
|
||||
## 4. Commit Guidelines
|
||||
|
||||
### Conventional Changelog Format
|
||||
|
||||
Every commit **must** follow the
|
||||
[Conventional Changelog standard](https://github.com/conventional-changelog/conventional-changelog-eslint/blob/master/convention.md).
|
||||
The format is:
|
||||
|
||||
```
|
||||
<type>(<scope>): <short summary>
|
||||
|
||||
<optional body — wrap at 72 characters>
|
||||
|
||||
<optional footer — ISSUES CLOSED: #N>
|
||||
```
|
||||
|
||||
**Allowed types:**
|
||||
|
||||
| Type | When to use |
|
||||
|------|-------------|
|
||||
| `feat` | New feature or user-visible behaviour |
|
||||
| `fix` | Bug fix |
|
||||
| `docs` | Documentation only |
|
||||
| `refactor` | Code restructuring without behaviour change |
|
||||
| `test` | Adding or updating tests |
|
||||
| `chore` | Build system, tooling, dependency updates |
|
||||
| `perf` | Performance improvement |
|
||||
| `ci` | CI/CD pipeline changes |
|
||||
|
||||
**Example:**
|
||||
|
||||
```
|
||||
fix(concurrency): guard execute_plan with advisory lock
|
||||
|
||||
LockService was implemented but never wired into the plan execution
|
||||
path, leaving execute_plan() unprotected against concurrent calls.
|
||||
|
||||
ISSUES CLOSED: #7989
|
||||
```
|
||||
|
||||
### Using Commitizen (recommended)
|
||||
|
||||
```bash
|
||||
# Instead of `git commit`, use:
|
||||
git cz
|
||||
```
|
||||
|
||||
Commitizen walks you through an interactive prompt to build a correctly
|
||||
formatted commit message. All CleverThis repositories ship a local
|
||||
`cz-customizable` configuration.
|
||||
|
||||
### Atomic Commits
|
||||
|
||||
- **One logical change per commit.** Never bundle unrelated changes.
|
||||
- **Include tests with the change.** Feature + tests are one logical unit.
|
||||
- **Include documentation with the change.** Update relevant docs in the same commit.
|
||||
- **Each commit must build and pass all tests.** The repository must be in a
|
||||
working state at every commit in history.
|
||||
|
||||
See [CONTRIBUTING.md](../../CONTRIBUTING.md) for the full commit quality policy.
|
||||
|
||||
### Branch Naming
|
||||
|
||||
Use the following prefixes:
|
||||
|
||||
| Prefix | Purpose |
|
||||
|--------|---------|
|
||||
| `feat/<issue>-<slug>` | New feature |
|
||||
| `fix/<issue>-<slug>` | Bug fix |
|
||||
| `docs/<slug>` | Documentation |
|
||||
| `refactor/<slug>` | Refactoring |
|
||||
| `chore/<slug>` | Tooling / maintenance |
|
||||
| `test/<slug>` | Test-only changes |
|
||||
|
||||
Examples:
|
||||
|
||||
```
|
||||
feat/issue-1234-plan-correction-rollback
|
||||
fix/7989-lock-service-concurrency
|
||||
docs/developer-setup-guide
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 5. Devcontainer Setup
|
||||
|
||||
The repository ships a [Dev Container](https://containers.dev/) configuration
|
||||
that provides a fully pre-configured development environment with Python 3.13,
|
||||
uv, nox, and all dependencies pre-installed. This is the **recommended** setup
|
||||
for contributors who want zero-friction onboarding.
|
||||
|
||||
### Requirements
|
||||
|
||||
- [Docker Desktop](https://www.docker.com/products/docker-desktop/) (macOS /
|
||||
Windows) or Docker Engine (Linux)
|
||||
- [VS Code](https://code.visualstudio.com/) with the
|
||||
[Dev Containers extension](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers),
|
||||
**or** the [devcontainer CLI](https://github.com/devcontainers/cli)
|
||||
|
||||
### Opening in VS Code
|
||||
|
||||
1. Open the repository folder in VS Code.
|
||||
2. When prompted *"Reopen in Container"*, click **Reopen in Container**.
|
||||
Alternatively: open the Command Palette (`Ctrl+Shift+P` / `Cmd+Shift+P`) and
|
||||
run **Dev Containers: Reopen in Container**.
|
||||
3. VS Code builds the container image and installs all extensions. This takes a
|
||||
few minutes on first run; subsequent opens are fast.
|
||||
4. A terminal inside the container is ready to use. All tools (`uv`, `nox`,
|
||||
`git`, `pre-commit`) are on the PATH.
|
||||
|
||||
### Using the devcontainer CLI
|
||||
|
||||
```bash
|
||||
# Start the container
|
||||
devcontainer up --workspace-folder .
|
||||
|
||||
# Execute a command inside the container
|
||||
devcontainer exec --workspace-folder . nox -s unit_tests
|
||||
```
|
||||
|
||||
### Named Configurations
|
||||
|
||||
The repository may define multiple named devcontainer configurations (e.g.,
|
||||
`default`, `docs-only`). To select one in VS Code, use
|
||||
**Dev Containers: Reopen in Container…** and choose the desired configuration
|
||||
from the list.
|
||||
|
||||
### Environment Variables
|
||||
|
||||
Sensitive configuration (API keys, database URLs) should be placed in a
|
||||
`.env` file at the repository root. The devcontainer automatically mounts
|
||||
this file. **Never commit `.env` to version control.**
|
||||
|
||||
Common variables:
|
||||
|
||||
| Variable | Description |
|
||||
|----------|-------------|
|
||||
| `CLEVERAGENTS_DATABASE_URL` | SQLite or PostgreSQL connection string |
|
||||
| `CLEVERAGENTS_HOME` | Override the default data directory |
|
||||
| `CLEVERAGENTS_TESTING_USE_MOCK_AI` | Set to `1` to use mock AI in tests |
|
||||
| `OPENAI_API_KEY` | OpenAI provider key (optional) |
|
||||
| `ANTHROPIC_API_KEY` | Anthropic provider key (optional) |
|
||||
|
||||
---
|
||||
|
||||
## 6. Troubleshooting
|
||||
|
||||
### `python3 --version` shows < 3.13
|
||||
|
||||
Your system Python is too old. Install Python 3.13 via pyenv, your OS package
|
||||
manager, or [python.org](https://www.python.org/downloads/), then recreate the
|
||||
virtual environment:
|
||||
|
||||
```bash
|
||||
uv venv --python 3.13
|
||||
source .venv/bin/activate
|
||||
uv pip install -e ".[dev,tests,docs]"
|
||||
```
|
||||
|
||||
### `nox` command not found
|
||||
|
||||
Install nox into your system Python or via uv tools:
|
||||
|
||||
```bash
|
||||
pip install nox
|
||||
# or
|
||||
uv tool install nox
|
||||
```
|
||||
|
||||
Ensure the tool installation directory is on your `PATH` (uv prints the path
|
||||
after installation).
|
||||
|
||||
### `pre-commit` hooks not running
|
||||
|
||||
Reinstall the hooks:
|
||||
|
||||
```bash
|
||||
pre-commit install --overwrite
|
||||
```
|
||||
|
||||
If hooks still do not run, verify that `.git/hooks/pre-commit` exists and is
|
||||
executable.
|
||||
|
||||
### Import errors after installing dependencies
|
||||
|
||||
Ensure you activated the virtual environment before running any commands:
|
||||
|
||||
```bash
|
||||
source .venv/bin/activate # macOS / Linux
|
||||
.venv\Scripts\Activate.ps1 # Windows
|
||||
```
|
||||
|
||||
Then verify the package is installed in editable mode:
|
||||
|
||||
```bash
|
||||
pip show cleveragents
|
||||
```
|
||||
|
||||
### `nox -s unit_tests` fails with `AmbiguousStep`
|
||||
|
||||
Two Behave step files define the same step pattern. Check the error message for
|
||||
the conflicting step text, then either rename one step or add a unique prefix.
|
||||
See the [Testing Guide](testing.md#unit-tests-behave) for step naming conventions.
|
||||
|
||||
### Coverage below 97%
|
||||
|
||||
Run the HTML coverage report to identify uncovered lines:
|
||||
|
||||
```bash
|
||||
nox -s coverage_report
|
||||
open build/htmlcov/index.html
|
||||
```
|
||||
|
||||
Write Behave scenarios targeting the uncovered code paths. Do **not** lower the
|
||||
threshold or add `# pragma: no cover` without explicit team agreement.
|
||||
|
||||
### `pyright` reports errors in generated or third-party stubs
|
||||
|
||||
Check `pyproject.toml` under `[tool.pyright]` for the `exclude` list. Generated
|
||||
stubs under `docs/reference/contracts/stubs/` are excluded by the ruff
|
||||
configuration and should also be excluded from pyright. Add a targeted
|
||||
`# type: ignore` with a comment only as a last resort.
|
||||
|
||||
### Robot Framework `Resource file does not exist`
|
||||
|
||||
Always use `${CURDIR}/` prefix for `Resource` imports:
|
||||
|
||||
```robot
|
||||
Resource ${CURDIR}/common.resource
|
||||
```
|
||||
|
||||
Bare paths are resolved relative to the working directory, not the test file.
|
||||
|
||||
### Devcontainer fails to start
|
||||
|
||||
1. Ensure Docker is running (`docker info`).
|
||||
2. Try **Dev Containers: Rebuild Container** from the VS Code Command Palette.
|
||||
3. Check the Docker build log in the VS Code Output panel for errors.
|
||||
4. If the issue persists, delete the container image and rebuild:
|
||||
```bash
|
||||
docker system prune -f
|
||||
devcontainer up --workspace-folder . --remove-existing-container
|
||||
```
|
||||
|
||||
### SQLite in-memory tests lose data between steps
|
||||
|
||||
When writing Behave tests with `sqlite:///:memory:`, use a single shared
|
||||
`Session` across all repository calls in a scenario — do not create a new
|
||||
session per call. See the [Testing Guide](testing.md#troubleshooting) for the
|
||||
correct pattern.
|
||||
|
||||
---
|
||||
|
||||
## See Also
|
||||
|
||||
- [Testing Guide](testing.md) — Full testing strategy, coverage requirements,
|
||||
and suite documentation
|
||||
- [CI/CD Pipeline](ci-cd.md) — Forgejo CI job definitions and pipeline stages
|
||||
- [Quality Automation](quality-automation.md) — Automated quality gates and
|
||||
enforcement
|
||||
- [CONTRIBUTING.md](../../CONTRIBUTING.md) — Commit standards, code of conduct,
|
||||
and contribution workflow
|
||||
- [Architecture Decision Records](../adr/index.md) — Key design decisions
|
||||
(ADR-003 DI, ADR-005 Tech Stack, ADR-009 Project Model, etc.)
|
||||
@@ -30,6 +30,7 @@ nav:
|
||||
- ACMS Context Hydration: modules/context-hydration.md
|
||||
- Git Worktree Sandbox: modules/git-worktree-sandbox.md
|
||||
- Development:
|
||||
- Setup Guide: development/setup.md
|
||||
- Agent System Specification: development/agent-system-specification.md
|
||||
- CI/CD Pipeline: development/ci-cd.md
|
||||
- Quality Automation: development/quality-automation.md
|
||||
|
||||
@@ -55,6 +55,7 @@ from cleveragents.application.services.fix_then_revalidate import (
|
||||
FixThenRevalidateOrchestrator,
|
||||
)
|
||||
from cleveragents.application.services.invariant_service import InvariantService
|
||||
from cleveragents.application.services.lock_service import LockService
|
||||
from cleveragents.application.services.multi_project_service import (
|
||||
MultiProjectService,
|
||||
)
|
||||
@@ -267,6 +268,24 @@ def _build_session_factory(database_url: str) -> sessionmaker[Session]:
|
||||
return sessionmaker(bind=engine, expire_on_commit=False)
|
||||
|
||||
|
||||
def _build_lock_service(
|
||||
database_url: str,
|
||||
) -> LockService:
|
||||
"""Build a LockService with a session factory from the database URL.
|
||||
|
||||
Registered as a Singleton so that all callers within a process share
|
||||
the same advisory-lock state. Follows the same ``_build_*`` pattern
|
||||
used by other DB-backed services (see ``_build_session_factory``,
|
||||
``_build_resource_registry_service``, etc.).
|
||||
"""
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
engine = create_engine(database_url, echo=False)
|
||||
factory = sessionmaker(bind=engine, expire_on_commit=False)
|
||||
return LockService(session_factory=factory)
|
||||
|
||||
|
||||
def _build_resource_registry_service(
|
||||
database_url: str,
|
||||
) -> ResourceRegistryService:
|
||||
@@ -657,6 +676,12 @@ class Container(containers.DeclarativeContainer):
|
||||
InvariantService,
|
||||
)
|
||||
|
||||
# Lock Service - Singleton (shared advisory-lock state per process, #7989)
|
||||
lock_service = providers.Singleton(
|
||||
_build_lock_service,
|
||||
database_url=database_url,
|
||||
)
|
||||
|
||||
# Plan Lifecycle Service - Factory (v3 four-phase lifecycle)
|
||||
plan_lifecycle_service = providers.Factory(
|
||||
PlanLifecycleService,
|
||||
@@ -665,6 +690,7 @@ class Container(containers.DeclarativeContainer):
|
||||
decision_service=decision_service,
|
||||
event_bus=event_bus,
|
||||
invariant_service=invariant_service,
|
||||
lock_service=lock_service,
|
||||
)
|
||||
|
||||
# Checkpoint Service - database-backed via CheckpointRepository
|
||||
|
||||
@@ -54,6 +54,7 @@ from __future__ import annotations
|
||||
from contextlib import suppress
|
||||
from datetime import datetime
|
||||
from typing import TYPE_CHECKING, Any
|
||||
from uuid import uuid4
|
||||
|
||||
import structlog
|
||||
from ulid import ULID
|
||||
@@ -101,6 +102,7 @@ if TYPE_CHECKING:
|
||||
ErrorPatternService,
|
||||
)
|
||||
from cleveragents.application.services.invariant_service import InvariantService
|
||||
from cleveragents.application.services.lock_service import LockService
|
||||
from cleveragents.config.settings import Settings
|
||||
from cleveragents.infrastructure.database.unit_of_work import UnitOfWork
|
||||
from cleveragents.infrastructure.events.protocol import EventBus
|
||||
@@ -190,6 +192,7 @@ class PlanLifecycleService:
|
||||
error_pattern_service: ErrorPatternService | None = None,
|
||||
config_service: ConfigService | None = None,
|
||||
invariant_service: InvariantService | None = None,
|
||||
lock_service: LockService | None = None,
|
||||
):
|
||||
"""Initialize the plan lifecycle service.
|
||||
|
||||
@@ -225,6 +228,13 @@ class PlanLifecycleService:
|
||||
Reconciliation Actor is auto-invoked at each phase
|
||||
transition to verify invariants hold. When ``None``,
|
||||
reconciliation is silently skipped.
|
||||
lock_service: Optional :class:`LockService` for plan-level
|
||||
advisory locking. When provided, ``execute_plan`` and
|
||||
``apply_plan`` acquire a plan lock before performing
|
||||
the phase transition and release it in a ``finally``
|
||||
block, preventing concurrent modifications to the same
|
||||
plan. When ``None``, locking is silently skipped for
|
||||
backward compatibility with existing tests.
|
||||
"""
|
||||
self.settings = settings
|
||||
self.unit_of_work = unit_of_work
|
||||
@@ -234,6 +244,7 @@ class PlanLifecycleService:
|
||||
self.error_pattern_service = error_pattern_service
|
||||
self._config_service = config_service
|
||||
self.invariant_service = invariant_service
|
||||
self._lock_service = lock_service
|
||||
self._logger = logger.bind(service="plan_lifecycle")
|
||||
self.preflight_guardrail = PlanPreflightGuardrail()
|
||||
self._subscribe_correction_reconciliation()
|
||||
@@ -1497,6 +1508,11 @@ class PlanLifecycleService:
|
||||
|
||||
This is the 'execute' command.
|
||||
|
||||
A plan-level advisory lock is acquired before the transition and
|
||||
released in a ``finally`` block so that concurrent callers on the
|
||||
same ``plan_id`` receive a ``LockConflictError`` rather than
|
||||
silently racing into the same phase transition.
|
||||
|
||||
Args:
|
||||
plan_id: The plan ULID
|
||||
|
||||
@@ -1507,67 +1523,91 @@ class PlanLifecycleService:
|
||||
NotFoundError: If plan not found
|
||||
InvalidPhaseTransitionError: If transition is not valid
|
||||
PlanNotReadyError: If plan is not ready for transition
|
||||
LockConflictError: If another session holds the plan lock
|
||||
"""
|
||||
plan = self.get_plan(plan_id)
|
||||
# Generate a unique owner_id for this invocation to ensure that
|
||||
# concurrent sessions cannot re-entrantly acquire the same lock.
|
||||
# The LockService treats owner_id as the caller identity and allows
|
||||
# re-entrant acquisition for the same owner; using a unique UUID per
|
||||
# invocation ensures that concurrent sessions present different owners
|
||||
# and thus trigger LockConflictError when attempting to acquire the
|
||||
# same plan lock.
|
||||
owner_id: str = str(uuid4())
|
||||
|
||||
# Validate transition
|
||||
if not can_transition(plan.phase, PlanPhase.EXECUTE):
|
||||
raise InvalidPhaseTransitionError(plan.phase, PlanPhase.EXECUTE)
|
||||
|
||||
# Must be in COMPLETE state to transition
|
||||
if plan.state != ProcessingState.COMPLETE:
|
||||
raise PlanNotReadyError(
|
||||
plan_id, plan.phase, plan.state or ProcessingState.QUEUED
|
||||
if self._lock_service is not None:
|
||||
self._lock_service.acquire(
|
||||
owner_id=owner_id,
|
||||
resource_type="plan",
|
||||
resource_id=plan_id,
|
||||
)
|
||||
try:
|
||||
plan = self.get_plan(plan_id)
|
||||
|
||||
# Run estimation actor if configured (informational only)
|
||||
self._run_estimation(plan)
|
||||
# Validate transition
|
||||
if not can_transition(plan.phase, PlanPhase.EXECUTE):
|
||||
raise InvalidPhaseTransitionError(plan.phase, PlanPhase.EXECUTE)
|
||||
|
||||
# Layer 4: Consult Error Pattern Database for preventive guidance
|
||||
self._consult_error_patterns(plan)
|
||||
# Must be in COMPLETE state to transition
|
||||
if plan.state != ProcessingState.COMPLETE:
|
||||
raise PlanNotReadyError(
|
||||
plan_id, plan.phase, plan.state or ProcessingState.QUEUED
|
||||
)
|
||||
|
||||
# Invariant Reconciliation: verify invariants before Execute
|
||||
self._run_invariant_reconciliation(plan)
|
||||
# Run estimation actor if configured (informational only)
|
||||
self._run_estimation(plan)
|
||||
|
||||
# Transition to Execute phase — set processing_state first so that
|
||||
# the phase-state validator sees QUEUED (valid in any phase) when
|
||||
# the phase assignment triggers re-validation.
|
||||
plan.processing_state = ProcessingState.QUEUED
|
||||
plan.phase = PlanPhase.EXECUTE
|
||||
plan.timestamps.updated_at = datetime.now()
|
||||
# Layer 4: Consult Error Pattern Database for preventive guidance
|
||||
self._consult_error_patterns(plan)
|
||||
|
||||
self._commit_plan(plan)
|
||||
self._logger.info(
|
||||
"Plan transitioned to Execute",
|
||||
plan_id=plan_id,
|
||||
phase=plan.phase.value,
|
||||
)
|
||||
if self.event_bus is not None:
|
||||
try:
|
||||
self.event_bus.emit(
|
||||
DomainEvent(
|
||||
event_type=EventType.PLAN_PHASE_CHANGED,
|
||||
plan_id=plan_id,
|
||||
details={
|
||||
"phase": plan.phase.value,
|
||||
"processing_state": plan.processing_state.value
|
||||
if plan.processing_state
|
||||
else None,
|
||||
},
|
||||
# Invariant Reconciliation: verify invariants before Execute
|
||||
self._run_invariant_reconciliation(plan)
|
||||
|
||||
# Transition to Execute phase — set processing_state first so that
|
||||
# the phase-state validator sees QUEUED (valid in any phase) when
|
||||
# the phase assignment triggers re-validation.
|
||||
plan.processing_state = ProcessingState.QUEUED
|
||||
plan.phase = PlanPhase.EXECUTE
|
||||
plan.timestamps.updated_at = datetime.now()
|
||||
|
||||
self._commit_plan(plan)
|
||||
self._logger.info(
|
||||
"Plan transitioned to Execute",
|
||||
plan_id=plan_id,
|
||||
phase=plan.phase.value,
|
||||
)
|
||||
if self.event_bus is not None:
|
||||
try:
|
||||
self.event_bus.emit(
|
||||
DomainEvent(
|
||||
event_type=EventType.PLAN_PHASE_CHANGED,
|
||||
plan_id=plan_id,
|
||||
details={
|
||||
"phase": plan.phase.value,
|
||||
"processing_state": plan.processing_state.value
|
||||
if plan.processing_state
|
||||
else None,
|
||||
},
|
||||
)
|
||||
)
|
||||
except Exception:
|
||||
self._logger.warning(
|
||||
"event_bus_emit_failed",
|
||||
event_type="PLAN_PHASE_CHANGED",
|
||||
plan_id=plan_id,
|
||||
exc_info=True,
|
||||
)
|
||||
)
|
||||
except Exception:
|
||||
self._logger.warning(
|
||||
"event_bus_emit_failed",
|
||||
event_type="PLAN_PHASE_CHANGED",
|
||||
plan_id=plan_id,
|
||||
exc_info=True,
|
||||
)
|
||||
|
||||
# Enqueue async job when async execution is enabled
|
||||
self._maybe_enqueue_async_job(plan_id, "execute")
|
||||
# Enqueue async job when async execution is enabled
|
||||
self._maybe_enqueue_async_job(plan_id, "execute")
|
||||
|
||||
return plan
|
||||
return plan
|
||||
finally:
|
||||
if self._lock_service is not None:
|
||||
self._lock_service.release(
|
||||
owner_id=owner_id,
|
||||
resource_type="plan",
|
||||
resource_id=plan_id,
|
||||
)
|
||||
|
||||
def start_execute(self, plan_id: str) -> Plan:
|
||||
"""Start the Execute phase processing."""
|
||||
@@ -1681,6 +1721,11 @@ class PlanLifecycleService:
|
||||
|
||||
This is the 'apply' command.
|
||||
|
||||
A plan-level advisory lock is acquired before the transition and
|
||||
released in a ``finally`` block so that concurrent callers on the
|
||||
same ``plan_id`` receive a ``LockConflictError`` rather than
|
||||
silently racing into the same phase transition.
|
||||
|
||||
Args:
|
||||
plan_id: The plan ULID
|
||||
|
||||
@@ -1691,41 +1736,65 @@ class PlanLifecycleService:
|
||||
NotFoundError: If plan not found
|
||||
InvalidPhaseTransitionError: If transition is not valid
|
||||
PlanNotReadyError: If plan is not ready for transition
|
||||
LockConflictError: If another session holds the plan lock
|
||||
"""
|
||||
plan = self.get_plan(plan_id)
|
||||
# Generate a unique owner_id for this invocation to ensure that
|
||||
# concurrent sessions cannot re-entrantly acquire the same lock.
|
||||
# The LockService treats owner_id as the caller identity and allows
|
||||
# re-entrant acquisition for the same owner; using a unique UUID per
|
||||
# invocation ensures that concurrent sessions present different owners
|
||||
# and thus trigger LockConflictError when attempting to acquire the
|
||||
# same plan lock.
|
||||
owner_id: str = str(uuid4())
|
||||
|
||||
# Validate transition
|
||||
if not can_transition(plan.phase, PlanPhase.APPLY):
|
||||
raise InvalidPhaseTransitionError(plan.phase, PlanPhase.APPLY)
|
||||
if self._lock_service is not None:
|
||||
self._lock_service.acquire(
|
||||
owner_id=owner_id,
|
||||
resource_type="plan",
|
||||
resource_id=plan_id,
|
||||
)
|
||||
try:
|
||||
plan = self.get_plan(plan_id)
|
||||
|
||||
# Must be in COMPLETE state to transition
|
||||
if plan.state != ProcessingState.COMPLETE:
|
||||
raise PlanNotReadyError(
|
||||
plan_id, plan.phase, plan.state or ProcessingState.QUEUED
|
||||
# Validate transition
|
||||
if not can_transition(plan.phase, PlanPhase.APPLY):
|
||||
raise InvalidPhaseTransitionError(plan.phase, PlanPhase.APPLY)
|
||||
|
||||
# Must be in COMPLETE state to transition
|
||||
if plan.state != ProcessingState.COMPLETE:
|
||||
raise PlanNotReadyError(
|
||||
plan_id, plan.phase, plan.state or ProcessingState.QUEUED
|
||||
)
|
||||
|
||||
# Invariant Reconciliation: verify invariants before Apply
|
||||
self._run_invariant_reconciliation(plan)
|
||||
|
||||
# Transition to Apply phase — set processing_state first so that
|
||||
# the phase-state validator sees QUEUED (valid in any phase) when
|
||||
# the phase assignment triggers re-validation.
|
||||
plan.processing_state = ProcessingState.QUEUED
|
||||
plan.phase = PlanPhase.APPLY
|
||||
plan.timestamps.apply_started_at = datetime.now()
|
||||
plan.timestamps.updated_at = datetime.now()
|
||||
|
||||
self._commit_plan(plan)
|
||||
self._logger.info(
|
||||
"Plan transitioned to Apply",
|
||||
plan_id=plan_id,
|
||||
phase=plan.phase.value,
|
||||
)
|
||||
|
||||
# Invariant Reconciliation: verify invariants before Apply
|
||||
self._run_invariant_reconciliation(plan)
|
||||
# Enqueue async job when async execution is enabled
|
||||
self._maybe_enqueue_async_job(plan_id, "apply")
|
||||
|
||||
# Transition to Apply phase — set processing_state first so that
|
||||
# the phase-state validator sees QUEUED (valid in any phase) when
|
||||
# the phase assignment triggers re-validation.
|
||||
plan.processing_state = ProcessingState.QUEUED
|
||||
plan.phase = PlanPhase.APPLY
|
||||
plan.timestamps.apply_started_at = datetime.now()
|
||||
plan.timestamps.updated_at = datetime.now()
|
||||
|
||||
self._commit_plan(plan)
|
||||
self._logger.info(
|
||||
"Plan transitioned to Apply",
|
||||
plan_id=plan_id,
|
||||
phase=plan.phase.value,
|
||||
)
|
||||
|
||||
# Enqueue async job when async execution is enabled
|
||||
self._maybe_enqueue_async_job(plan_id, "apply")
|
||||
|
||||
return plan
|
||||
return plan
|
||||
finally:
|
||||
if self._lock_service is not None:
|
||||
self._lock_service.release(
|
||||
owner_id=owner_id,
|
||||
resource_type="plan",
|
||||
resource_id=plan_id,
|
||||
)
|
||||
|
||||
def start_apply(self, plan_id: str) -> Plan:
|
||||
"""Start the Apply phase processing."""
|
||||
|
||||
Reference in New Issue
Block a user