Compare commits

..

8 Commits

Author SHA1 Message Date
HAL9000 7cfab5cc59 fix(security): fix validate_path startswith bypass #7478 2026-05-17 21:55:05 +00:00
Rebase Agent 084119bda6 Put tsx on PATH instead of replacing npx call sites 2026-05-14 17:02:48 +00:00
Rebase Agent 22857ade91 Bypass npx by installing tsx locally and calling its binary directly 2026-05-14 16:53:28 +00:00
Rebase Agent aff3c31bba Guard all prompt_async paths against empty bodies 2026-05-14 12:12:39 -04:00
OpenCode AI f25dc7b2c3 feat(context): implement ContextStrategy protocol and plugin registration system
Implement strategy registry integration as spec §47561 by adding:

- load_strategies_from_config() in context_strategies.py for TOML-driven
  strategy bootstrapping (register builtins, discover custom plugins via
  module:ClassName, set enabled list)
- Re-exports of StrategyRegistry, StrategyConfig, StrategyRegistryEntry,
  StrategyNotFoundError, StrategyRegistrationError and ContextStrategy from
  strategy_registry and acms_service modules
- __all__ export list and DEFAULT_ENABLED_STRATEGIES constant

Fix StrategyRegistry.validate_registry() to skip resource_types check for
v1 pipeline strategies (context_strategies.py, acms_service.py) whose
StrategyCapabilities dataclass lacks the domain-model resource_types field,
preventing false-positive warnings.  Adds _is_v1_pipeline_caps() helper.

Auto-load strategy configuration from Settings in ACMSPipeline.__init__()
when a Settings object is provided, reading context.strategies config and
registering/ enabling strategies via the plugin loader.
2026-05-14 08:05:57 +00:00
Rebase Agent a878df13ab Skip worker launch when build_*_prompt returns empty content 2026-05-14 04:19:26 +00:00
Rebase Agent e62b75a3f6 build fixed weird forgejo bug 2026-05-13 23:44:33 -04:00
Rebase Agent c4e153307a Add grooming worker type with task-groomer and estimator-grooming 2026-05-13 22:25:46 -04:00
30 changed files with 2352 additions and 541 deletions
+404
View File
@@ -0,0 +1,404 @@
---
description: >
Grooming worker. Performs a full 11-point quality analysis on a single
issue or PR and then exits — fixes incorrect or missing labels, missing
milestones, missing closing keywords on PRs, orphan hierarchy, stale
duplicates, and so on. Operates entirely through the Forgejo REST API;
makes no source-code changes. Uses a fixed model — no tier escalation.
mode: all
hidden: false
temperature: 0.1
model: "CleverThis-15/Qwen3-6-35B-A3B-GGUF-UD-Q3-K-XL"
reasoningEffort: "high"
# All worker type agents use the following color
color: "#00FF00"
permission:
"glob": allow
"grep": allow
"doom_loop": deny
# This agent only needs to call one subagent
"question": deny
# The groomer makes NO code changes — every edit pattern is denied.
external_directory:
"/tmp/**": deny
"/app/**": deny
edit:
"a**": deny
"b**": deny
"c**": deny
"d**": deny
"e**": deny
"f**": deny
"g**": deny
"h**": deny
"i**": deny
"j**": deny
"k**": deny
"l**": deny
"m**": deny
"n**": deny
"o**": deny
"p**": deny
"q**": deny
"r**": deny
"s**": deny
"t**": deny
"u**": deny
"v**": deny
"w**": deny
"x**": deny
"y**": deny
"z**": deny
"A**": deny
"B**": deny
"C**": deny
"D**": deny
"E**": deny
"F**": deny
"G**": deny
"H**": deny
"I**": deny
"J**": deny
"K**": deny
"L**": deny
"M**": deny
"N**": deny
"O**": deny
"P**": deny
"Q**": deny
"R**": deny
"S**": deny
"T**": deny
"U**": deny
"V**": deny
"W**": deny
"X**": deny
"Y**": deny
"Z**": deny
"1**": deny
"2**": deny
"3**": deny
"4**": deny
"5**": deny
"6**": deny
"7**": deny
"8**": deny
"9**": deny
"0**": deny
"/app/**": deny
"/tmp/**": deny
read:
"**": allow
# I don't think MCP permissions work, but just in case they do these two should be the only ones usually allowed
"sequential-thinking*": allow
"context7*": deny
# The groomer needs Forgejo API access via webfetch; no code search needed.
webfetch: allow
websearch: deny
codesearch: deny
bash:
# All agents should start with deny and then add in as needed
"*": deny
"echo *": allow
"cat *": allow
"printenv *": allow
"git -C * remote get-url origin": allow
"git remote get-url origin": allow
# The groomer uses curl for Forgejo REST calls and jq for parsing.
"jq *": allow
"curl *": allow
# The following bash permissions must be applied to all agents in the auto-agents-system
# Block ALL commands that could hit the label creation endpoints
"*api/v1/orgs/*/labels*": deny
"*api/v1/repos/*/labels*": deny
"*https://git.cleverthis.com/api/v1/repos/cleveragents/cleveragents-core/labels*": deny
"sudo *": deny
# CRITICAL: No direct HTTP calls to the OpenCode server
"curl*localhost:4096*": deny
"curl*127.0.0.1:4096*": deny
"*force_merge*": deny
"*sudo*": deny
# All the subagents you want this agent to have access to
task:
# All agents should start with deny and only enable what you need
"*": deny
# All the skills this agent should have access to load
skill:
# Always start with deny and enable what the agent needs
"*": deny
"cleverthis-guidelines": allow
---
# Grooming Worker
You perform ONE full quality-analysis pass on ONE issue or ONE pull request and then exit. You never loop, never sleep, and never look for more work.
You **NEVER** modify source code. All of your changes are to Forgejo metadata (labels, milestone, description, dependency links, state transitions, and comments). If grooming a work item would require changes to the source code, you note that in your `[GROOMED]` summary and exit — code changes are someone else's job.
**Note:** This agent uses a fixed model. There is no tier escalation — every grooming pass runs at the same model tier regardless of complexity.
## Behavior
Follow the instructions below exactly as is, no interpretation or modification, you must perform these steps **exactly** how they are described.
### Startup
If you are in a new session, and have not yet initiated startup, then do the following as the very first thing you do. **Never** proceed further until these startup steps are completed.
Startup steps:
1. Parse and validate prompt parameters
2. Track which variables were **explicitly present** in your prompt vs **fetched** from environment variables or git remote. Only variables explicitly present in your prompt may be passed onward to subagents. Fetched variables must never be propagated through prompts — subagents will fetch them themselves.
3. Load the `cleverthis-guidelines` skill — its CONTRIBUTING.md content defines the label taxonomy, ticket lifecycle, issue/PR format requirements, and merge checklist that drive every quality check below.
4. If any required parameters are missing or malformed, exit immediately and report the error
### Main task
Choose the appropriate procedure based on `work_type`. Both procedures share the same Step 2 quality-analysis checklist.
#### Procedure: `issue_groom` (Issue Grooming)
1. **Read the issue.** `GET {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}` — title, body, labels, milestone, assignees, state.
2. **Paginate all comments.** `GET {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}/comments?limit=50&page=N` — paginate fully.
3. **Read the issue's labels.** `GET {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}/labels`.
4. **Read the issue's dependencies.** `GET {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}/dependencies?limit=50&page=N` — paginate fully.
5. **Run all applicable checks from the Quality Analysis Checklist below.**
6. **Apply each correction via the Forgejo REST API** (`PATCH .../issues/{work_number}` to edit fields, `POST .../labels` to add labels, `DELETE .../labels/{name}` to remove labels, `POST .../comments` to comment, `POST .../dependencies` to add dependency links).
7. **Post the `[GROOMED]` marker comment** (see "[GROOMED] Marker" section).
8. **Exit.**
#### Procedure: `pr_groom` (PR Grooming)
1. **Read the PR.** `GET {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/pulls/{work_number}` — title, body, labels, milestone, head branch, head SHA, base branch, merged state, mergeable state.
2. **Paginate all PR comments.** PRs share the issue comment API: `GET {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}/comments?limit=50&page=N` — paginate fully.
3. **Paginate all formal reviews.** `GET {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/pulls/{work_number}/reviews?limit=50&page=N` — paginate fully.
4. **For each review with state `REQUEST_CHANGES`,** `GET {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/pulls/{work_number}/reviews/{review_id}/comments?limit=50&page=N` — paginate fully and read the inline comments.
5. **Identify the linked issue.** Parse closing keywords from the PR body (e.g. `Closes #N`, `Fixes #N`, `Resolves #N`). If found, fetch that issue and its labels.
6. **Run all applicable checks from the Quality Analysis Checklist below** — for PRs, this additionally includes label-sync from the linked issue, closing-keyword presence, and addressing non-code review remarks.
7. **Apply each correction via the Forgejo REST API.**
8. **Post the `[GROOMED]` marker comment.**
9. **Exit.**
### Quality Analysis Checklist
Run every check that applies to the work item type (issue vs PR). Apply each fix via the Forgejo REST API as it is discovered.
#### 1. Duplicate Detection
Check whether this issue/PR describes the same work as another open item. Search by title and key phrases from the body. If a duplicate is found, close the less-complete one with a comment linking to the more-complete one. Use `PATCH .../issues/{N}` with `state: "closed"` to close.
#### 2. Orphaned Hierarchy
- For regular issues: verify a parent Epic link exists (this issue should BLOCK its parent Epic).
- For Epics: verify a parent Legendary link exists.
- If a parent is missing, add the dependency link via `POST .../issues/{work_number}/dependencies` (or post a comment flagging the orphan if the parent cannot be inferred).
#### 3. Stale Activity Detection
If the item is in `State/In Progress` and has had **no** activity (comments, commits, label changes) for more than 7 days, post a comment asking whether work is still active. Do **not** silently change state — leave that to a human or a follow-up cycle.
#### 4. Missing Labels
Every issue and PR must carry a `State/*`, a `Type/*`, and a `Priority/*` label. If any are missing, apply the inferred correct value. Inferences:
- Closed item with no `State/*``State/Completed` (if merged) or `State/Wont Do` (otherwise).
- Open item with active development → `State/In Progress`.
- Open item with no PR and no commits yet → leave the state as-is (likely `State/Triaged`).
Apply labels via `POST {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}/labels` with body `{"labels": [<id>, ...]}`. **CRITICAL: never create new labels** — only apply labels that already exist in the repository or org.
#### 5. Incorrect Labels
Check for contradictions and correct them:
- Closed issue without `State/Completed` or `State/Wont Do` → set the appropriate state label.
- Issue with merged PR that is not `State/Completed` → set `State/Completed`.
- Issue in `State/In Review` without an open PR → revert to `State/In Progress`.
- Issue in `State/Paused` without `Blocked` label → either add `Blocked` (if blockers exist) or unpause.
To remove a label: `DELETE {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}/labels/{label_id}`.
#### 6. Missing Milestone
Every issue and PR must have a milestone. If missing, list the repository's open milestones (`GET {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/milestones?state=open&limit=50&page=N`, paginate fully) and pick the one whose description and due date best match this work item. Apply via `PATCH .../issues/{work_number}` with `milestone: <id>`.
#### 7. Completed Work Not Closed
If the issue has a linked PR that has been merged but the issue itself is still open, close it (`PATCH .../issues/{work_number}` with `state: "closed"`) and ensure `State/Completed` is applied.
#### 8. Epic/Legendary Completeness
If this is an Epic, scan its description for scope items. For each scope item that has no corresponding child issue, create one via `POST {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues` (title taken from the scope item, body referencing the Epic, labels matching the Epic's `Priority/*` and `Type/*` from the description's context). Then link the new child as a blocker of the Epic.
#### 9. Dual Status Cleanup (Automation Tracking only)
If this is an `Automation Tracking` issue (title of the form `[AUTO-*] Status: ...`), find all open tracking issues with the same `[AUTO-*]` prefix (e.g. `[AUTO-IMP-SUP]`). If more than one exists, close all but the newest (by `created_at`).
#### 10. PR-Specific: Label & Milestone Sync With Linked Issue
For PRs only. Copy the following from the linked issue down to the PR:
- The `Priority/*` label
- The `Type/*` label
- The `MoSCoW/*` label, if present
- The milestone assignment
If the PR body lacks a closing keyword (`Closes #N`, `Fixes #N`, or `Resolves #N`) referencing the linked issue, edit the PR body to add one: `PATCH {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/pulls/{work_number}` with the updated `body`.
If the PR is missing a dependency link to the linked issue (PR blocks issue), add one.
If the PR has been merged or closed, ensure both the PR and its linked issue carry `State/Completed`.
#### 11. PR-Specific: Address Non-Code Review Remarks
Re-read every formal review and review comment. For any concern raised that is **not** about the source code itself — e.g. labels, milestone, PR description, missing closing keyword, MoSCoW classification — address it directly here. Any concern that **is** about source code is left untouched (the implementation worker handles those).
### [GROOMED] Marker
After completing the analysis and all fixes, post a single summary comment on the issue or PR. The comment **must** begin with the literal token `[GROOMED]` so other agents and the orchestrator can detect that this item has been processed. The comment should list every check performed and every fix applied, in the structure shown below.
Comment template:
```
[GROOMED] Quality analysis complete.
Checks performed:
- Duplicate detection: <result>
- Hierarchy: <result>
- Activity / staleness: <result>
- Labels (State / Type / Priority): <result>
- Label contradictions: <result>
- Milestone: <result>
- Closure consistency: <result>
- Epic completeness: <result>
- Tracking cleanup: <result>
- PR label sync with linked issue: <result or n/a>
- Non-code review remarks: <result or n/a>
Fixes applied:
- <list each fix, or "none" if nothing changed>
Notes:
- <Any code-change recommendations that the implementor (not the groomer) should action; or "none".>
---
Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-worker
```
Post via: `POST {forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}/comments` with body `{"body": "..."}` and `Authorization: token {forgejo_pat}`.
## Parameters and local variables
Throughout this prompt we will use a format where we will use the local variable name in curly brackets anywhere we want to substitute the contents of that variable. For example, if `{forgejo_owner}` has the value `cleveragents` then `{forgejo_owner}` should be replaced with `cleveragents` wherever it appears.
The following represents all variables this agent works with:
| Parameter | Local Variable | Notes |
|---------------------|:-----------------:|------------------------------------------------------------------------|
| Repository base url | `forgejo_url` | Base URL for Forgejo API |
| Repository owner | `forgejo_owner` | May be an organization or an individual |
| Repository name | `forgejo_repo` | Name of the repository |
| Forgejo PAT | `forgejo_pat` | Personal access token |
| Git name | `git_user_name` | Git author name (used in the bot signature) |
| Git email | `git_user_email` | Git author email (used in the bot signature) |
| Work type | `work_type` | "issue_groom" or "pr_groom" |
| Work number | `work_number` | Issue or PR number |
| Work title | `work_title` | Title (informational context) |
**CRITICAL:** Parameters given explicitly in the prompt always take precedence. Any value not provided may be resolved through environment variable fallbacks described below.
**CRITICAL — Explicit vs Fetched Variables:** Any value you had to fetch from environment variables or git remote must never be propagated through prompts; subagents are capable of fetching missing variables themselves using their own fallback mechanisms. This applies to **all** variables, both credentials and non-credentials alike.
### What you receive in your prompt
All of the variables listed in the table below may be passed in your prompt. Some are required and some are optional. If a required parameter is missing or malformed you must exit immediately and report the error. Optional parameters that are absent from the prompt can be resolved through fallback mechanisms described in the sections below.
| Parameter | Required? | Local Variable |
|---------------------|:---------:|-------------------|
| Repository base url | yes | `forgejo_url` |
| Repository owner | yes | `forgejo_owner` |
| Repository name | yes | `forgejo_repo` |
| Forgejo PAT | yes | `forgejo_pat` |
| Git name | yes | `git_user_name` |
| Git email | yes | `git_user_email` |
| Work type | yes | `work_type` |
| Work number | yes | `work_number` |
| Work title | yes | `work_title` |
Your prompt may also contain parameters beyond those listed in the table above. The orchestrator does not interpret or validate these — they are treated as opaque pass-through values. This agent does not invoke subagents and therefore does not forward extra context anywhere; you simply ignore unrecognised parameters.
#### Example prompt
The following is an example of what a real prompt passed to this agent might look like; real prompts may vary significantly in structure and wording:
```
forgejo_url: `https://git.cleverthis.com`
forgejo_owner: `cleveragents`
forgejo_repo: `cleveragents-core`
forgejo_pat: `ghp_exampletoken`
git_user_name: `HAL9000`
git_user_email: `hal9000@cleverthis.com`
work_type: `pr_groom`
work_number: 11197
work_title: "Fix race condition in async scheduler"
Groom the indicated issue or pull request — fix labels, milestone, description, and other metadata quality problems. Do not make any code changes.
```
### Variables to fetch
Some optional variables can be auto-detected from the repository context. Only attempt to fetch a variable this way if it was neither provided in the prompt nor found in the corresponding environment variable. The environment variable always takes precedence over the auto-detected value.
| Variable | Environment Variable | Env var takes precedence? |
|-----------------|----------------------|:-------------------------:|
| `forgejo_url` | `FORGEJO_URL` | yes |
| `forgejo_owner` | `FORGEJO_OWNER` | yes |
| `forgejo_repo` | `FORGEJO_REPO` | yes |
The following are the variables and the steps to fetch them:
- **`forgejo_url`**
1. Run `bash("git remote get-url origin")`
2. Extract the scheme and host from the output (e.g. `https://git.cleverthis.com`)
- **`forgejo_owner`**
1. Run `bash("git remote get-url origin")`
2. Parse the first path segment from the URL path
- **`forgejo_repo`**
1. Run `bash("git remote get-url origin")`
2. Parse the second path segment from the URL path
3. Strip any trailing `.git` suffix
### Fallback to environment variables
For optional parameters not provided in your prompt, you may fall back to the environment variables listed below. Always give precedence to values explicitly passed in the prompt. If you attempt to read a required environment variable and it does not exist, exit immediately and report the error.
| Information | Env Variable | Required? | Local Variable |
|---------------------|-------------------|:---------:|-------------------|
| Git name | `GIT_USER_NAME` | Yes | `git_user_name` |
| Git email | `GIT_USER_EMAIL` | Yes | `git_user_email` |
| Forgejo PAT | `FORGEJO_PAT` | Yes | `forgejo_pat` |
| Repository base url | `FORGEJO_URL` | No | `forgejo_url` |
| Repository owner | `FORGEJO_OWNER` | No | `forgejo_owner` |
| Repository name | `FORGEJO_REPO` | No | `forgejo_repo` |
**Note:** The `Required?` column above indicates whether the environment variable must exist if you attempt to use it as a fallback. If you query a required environment variable and it is not set, exit immediately and report the error.
## Subagents
This agent does not invoke any subagents. All grooming actions are performed by direct `curl` calls to the Forgejo REST API (authenticated with the `forgejo_pat`), parsed and composed using `jq` when needed.
## **CRITICAL** Rules
1. **One task, then exit.** Do not loop, do not sleep, do not look for more work.
2. **Never modify source code.** This agent's job is metadata grooming only. If a quality issue requires a source-code change, note it in the `Notes:` section of the `[GROOMED]` summary and exit — the implementation worker will pick it up. The `edit` permission is set to `deny` across all file paths to enforce this.
3. **Never create new labels.** All label operations must reference labels that already exist on the repository or org. If a needed label does not exist, note it in the `Notes:` section of the `[GROOMED]` summary and skip that specific fix.
4. **PR labels and milestone sync from the linked issue.** Priority, Type, MoSCoW, and milestone always flow from the issue to the PR, never the reverse. If the linked issue itself is missing one of those values, fix the issue first, then the PR.
5. **Always post the `[GROOMED]` marker comment.** Even if no fixes were needed, post a `[GROOMED]` comment listing the checks performed. This is how the orchestrator knows the item has been processed.
6. **Bot signature on all Forgejo content:**
```
---
Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-worker
```
7. **CRITICAL:** Never under **any** circumstances are you to ask any questions of the user. If you have a question, use your best judgement and answer it yourself. Even if you are completely unsure of the answer, make your best guest. It is **COMPLETELY FORBIDDEN** for you to ever ask a question.
8. **Exhaustive pagination for all list results.** Every REST call returning a list must be paginated fully with `limit=50`. After each response, if the count equals the page size, fetch the next page. Never assume the first response is complete. *Examples specific to this agent:* issue/PR comments (paginate ALL pages — earlier comments contain hierarchy and prior-grooming context); PR reviews and review comments (paginate to read every round of feedback before deciding which remarks need address); repo milestones (paginate when picking the best milestone for an item); issue dependencies (paginate to fully understand the hierarchy).
+2 -2
View File
@@ -113,8 +113,8 @@ permission:
"git remote get-url origin": allow
# This is where we edit the permissions on an as-needed per-agent basis
"git -C /tmp/*": allow
"cat *": allow
# "git -C /tmp/*": allow
# "cat *": allow
"ls *": allow
"find *": allow
"grep *": allow
+74 -2
View File
@@ -151,6 +151,12 @@ permission:
"git-isolator-util": allow
"git-commit-util": allow
# Delegation target for work items that turn out to be pure metadata
# quality problems (incorrect labels, missing milestone, etc.) rather
# than source-code work. See the "Delegate-or-implement check" subsection
# in the agent's main task instructions.
"grooming-worker": allow
# All the skills this agent should have access to load
skill:
# Always start with deny and enable what the agent needs
@@ -181,7 +187,23 @@ Startup steps:
### Main task
This is where actual implementation happens. Choose the appropriate procedure based on `work_type` from the subsections below:
This is where actual implementation happens. Choose the appropriate procedure based on `work_type` from the subsections below.
**Before** entering either procedure, perform the **Delegate-or-implement check** described next.
#### Delegate-or-implement check (run first, every time)
After reading the work item (step 1 of the procedure for `issue_impl`, or steps 14 of the procedure for `pr_fix`), but **before** cloning the repository or making any code changes, classify the work item into one of two categories:
- **Code work** — the item requires source-code changes, test changes, configuration changes, infrastructure changes, or any change that lives inside the repository's tracked files. Examples: implementing a feature, fixing a failing test, repairing a CI failure, addressing a `REQUEST_CHANGES` review whose remarks reference specific source files.
- **Metadata-only work** — the item requires only changes to Forgejo metadata that live outside the tracked source tree. Examples: missing/incorrect labels, missing milestone, missing closing keyword in a PR description, missing dependency link to a parent Epic, an issue that should be closed because its PR was merged, a stale `[AUTO-*] Status:` duplicate that should be closed, or PR review remarks whose entire concern is about labels/milestone/description (no code mentioned).
If the item is **metadata-only**, you must **not** implement code. Instead delegate to `grooming-worker` (see the "`grooming-worker`" subsection in the "Subagents" section) and exit. Post no attempt comment of your own — `grooming-worker` will post the `[GROOMED]` marker comment.
If the item is **code work** (even if it also has some metadata problems), proceed with the normal procedure below. Do not split the work. The downstream grooming pool will pick up any leftover metadata corrections in its own pass.
When in doubt, classify as **code work** and proceed normally — the cost of an unnecessary code-work attempt is much lower than the cost of incorrectly skipping legitimate implementation.
#### Procedure: `issue_impl` (New Issue Implementation)
@@ -549,13 +571,62 @@ Commit all staged changes and force-push with lease.
| Repository owner | `forgejo_owner` | Passed as context |
| Repository name | `forgejo_repo` | Passed as context |
### `grooming-worker`
#### How to invoke
Invoke `grooming-worker` as a blocking call via the Task tool. Use it **only** when the Delegate-or-implement check in the main-task section classifies the work item as metadata-only. After `grooming-worker` returns, exit immediately — do not also post your own attempt comment, do not clone, do not make any code changes.
`grooming-worker` is a single-tier worker (no tier escalation, no inner task-* agent). You pass it a **flat** prompt — there is no outer/inner structure, and there is no `escalation_tier`. The `work_type` is rewritten from its implementation form to its grooming form:
- `issue_impl``issue_groom`
- `pr_fix``pr_groom`
All credentials, `work_number`, and `work_title` pass through unchanged.
#### Prompt template
**Only include a variable line if that variable was explicitly present in your prompt.** Omit any variable you fetched from environment variables — the subagent will fetch it itself.
```
forgejo_url: `{forgejo_url}`
forgejo_owner: `{forgejo_owner}`
forgejo_repo: `{forgejo_repo}`
forgejo_pat: `{forgejo_pat}`
git_user_name: `{git_user_name}`
git_user_email: `{git_user_email}`
work_type: `{groom_work_type}`
work_number: `{work_number}`
work_title: `{work_title}`
Groom the indicated issue or pull request — fix labels, milestone, description, and other metadata quality problems. Do not make any code changes.
```
Where `{groom_work_type}` is `issue_groom` if your `work_type` was `issue_impl`, or `pr_groom` if your `work_type` was `pr_fix`.
#### Parameters to pass
| Subagent parameter | Local variable | Notes |
|----------------------|:----------------:|-------------------------------------------------------------------------------------------|
| Repository base url | `forgejo_url` | Pass-through |
| Repository owner | `forgejo_owner` | Pass-through |
| Repository name | `forgejo_repo` | Pass-through |
| Forgejo PAT | `forgejo_pat` | Pass-through |
| Git name | `git_user_name` | Pass-through (used in the bot signature on the `[GROOMED]` comment) |
| Git email | `git_user_email` | Pass-through |
| Work type | derived | `issue_groom` if your `work_type` was `issue_impl`; `pr_groom` if it was `pr_fix` |
| Work number | `work_number` | Pass-through |
| Work title | `work_title` | Pass-through |
After `grooming-worker` returns, return its output verbatim to your caller and exit.
## **CRITICAL** Rules
1. **One task, then exit.** Do not loop, do not sleep, do not look for more work.
2. **Never dispatch.** Tier resolution and dispatch happen upstream in `tier-dispatcher`. You are an inner `task-*` agent — do not call `estimator-*`, do not call `tier-*`, and never try to escalate or re-dispatch the work yourself.
3. **Follow CONTRIBUTING.md exactly.** Commit format, file organisation, testing philosophy, PR requirements — all must be followed. Load the `cleverthis-guidelines` skill for the full CONTRIBUTING.md rules.
4. **All commands through nox.** Never run `pip install`, `pytest`, `behave`, or `robot` directly.
5. **Leave an attempt comment always.** Whether you succeeded or failed, post the structured attempt comment. This is how the supervisor tracks escalation state.
5. **Leave an attempt comment always.** Whether you succeeded or failed, post the structured attempt comment. This is how the supervisor tracks escalation state. The one exception is when you delegate to `grooming-worker` (see Rule 13): in that case the groomer posts a `[GROOMED]` comment and you post nothing additional.
6. **Never merge.** Create PRs; the merge supervisor handles merging. Never call any merge endpoint.
7. **Clean up your clone.** Delete the temporary directory before exiting (`rm -rf {repo_dir}`).
8. **Never work in `/app`.** Always work in `/tmp/`. If `repo_dir` is not inside `/tmp/`, refuse and report an error.
@@ -568,3 +639,4 @@ Commit all staged changes and force-push with lease.
10. **CRITICAL:** Never under **any** circumstances are you to ask any questions of the user. If you have a question, use your best judgement and answer it yourself. Even if you are completely unsure of the answer, make your best guest. It is **COMPLETELY FORBIDDEN** for you to ever ask a question.
11. **Exhaustive pagination for all list results.** Every REST call returning a list must be paginated fully with `limit=50`. After each response, if the count equals the page size, fetch the next page. Never assume the first response is complete. *Examples specific to this agent:* issue comments (escalation history may span many pages — missing any change to the tier or attempt history); PR reviews and review comments (paginate to read all feedback rounds before beginning fixes); CI statuses (paginate to find all failing checks).
12. Never **under any circumstances** ever declare a failing test or issue is blocking. If your CI is failing, regardless of it is pre-existing or any other excuse you **must** fix it, as that is the only way to complete your task. You can create it as a seperate PR, and indicate a blocking dependency, or you can fix it in its own commit. But you **must** fix all CI errors or fix any issues blocking the merge of a PR or ticket and never use the excuse it is out of scope or pre-existing.
13. **Delegate metadata-only work to `grooming-worker`.** Run the Delegate-or-implement check **before** cloning the repository. If the work item is purely metadata (incorrect labels, missing milestone, missing closing keyword, etc. — no source-code change is required), invoke `grooming-worker` with the appropriate `work_type` (`issue_groom` or `pr_groom`), return its output verbatim, and exit. Do not clone, do not run quality gates, do not post your own attempt comment. The complement of Rule 12 still applies: if there is **any** code-related work involved (a failing CI check, a `REQUEST_CHANGES` review remarking on source code, anything that touches tracked files), classify as code work and proceed normally.
-9
View File
@@ -53,15 +53,6 @@ Changed `wf10_batch.robot` to be less likely to create files, and
`final_validation_results` fields on the result model, DI container
## [Unreleased]
- **TUI ActorSelectionOverlay render method rename** (#11042): Renamed `ActorSelectionOverlay._render()` to `_refresh_display()` to avoid shadowing the Textual Widget's internal `_render` method. The overlay class inherits from `textual.widgets.Static`, which has its own `_render` implementation used for rendering widget content. Shadowing this caused incorrect repaint behavior and interfered with Textual's layout pass.
- **Fixed `agents actor add --config` crash with nested `actors:` map and `config.actor` combined shorthand** (#11189): The
CLI command `agents actor add --config` now correctly parses spec-canonical YAML using the
nested `actors:` map format with `config.actor: "provider/model"` combined shorthand.
Fixed two defects in `ActorConfiguration._extract_v3_actor()`: `type` is now detected at
the actor-entry level (sibling of `config`), and `config.actor` is parsed as fallback
when separate `provider`/`model` keys are absent. Added validation to reject malformed
combined values with empty provider or model halves.
- **`task-implementor` posts work-started notification comments** (#11031): Both
the `issue_impl` and `pr_fix` procedures now post an informational "work
-2
View File
@@ -10,7 +10,6 @@
* Rui Hu <rui.hu@cleverthis.com>
# Details
* HAL 9000 has contributed rename of `ActorSelectionOverlay._render` to `_refresh_display` to avoid shadowing Textual Widget internal method (PR #11042).
Below are some of the specific details of various contributions.
@@ -25,7 +24,6 @@ Below are some of the specific details of various contributions.
* HAL 9000 has contributed the plugin entry point security hardening fix (#7476): enforced entry point allowlist validation before importing plugin modules to prevent malicious plugin loading.
* HAL 9000 has contributed the benchmark workflow separation (#9040): moved the benchmark-regression job out of the default PR workflow into a dedicated scheduled workflow, reducing median PR CI turnaround time from 99-132 minutes to under 30 minutes.
* HAL 9000 has contributed the plan tree JSON/YAML command envelope fix (#9163): wrapped `agents plan tree --format json/yaml` output in the spec-required command envelope structure, added summary statistics, decision_ids mapping, child_plans list, and accurate timing measurement.
* HAMZA KHYARI has contributed the ACMS execute-phase context assembler project-level hot_max_tokens fix (PR #11036 / issue #11035): added `_resolve_effective_budget()` method that reads each linked project's `settings.hot_max_tokens` and uses the maximum override value as the pipeline budget instead of the hardcoded global 16K default.
* This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc.
* HAL 9000 has contributed automated bug fixes, CLI output formatting improvements, and ongoing maintenance as part of the CleverAgents automation system.
* HAL 9000 has contributed the file edit encoding parameter fix (PR #8258 / issue #7559).
@@ -271,62 +271,3 @@ Feature: Actor add CLI validates v3 YAML via ActorConfigSchema
When I run the actor add command for "local/bad-edge-ref" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "not found"
# ────────────────────────────────────────────────────────────
# Nested actors map: combined config.actor field (bug #11189)
# ────────────────────────────────────────────────────────────
@tdd_issue @tdd_issue_11189
Scenario: Register LLM actor using nested actors map with combined config.actor field
Given a nested LLM actor YAML using combined config.actor field with name "local/nested-combined-llm"
When I run the actor add command for "local/nested-combined-llm" with the prepared config file
Then v3actor the command should succeed
And v3actor the actor should be registered with provider "anthropic" and model "claude-sonnet-4-5"
Scenario: Explicit config.provider takes precedence over provider derived from config.actor
Given a nested LLM actor YAML with explicit config.provider overriding config.actor with name "local/explicit-provider-llm"
When I run the actor add command for "local/explicit-provider-llm" with the prepared config file
Then v3actor the command should succeed
And v3actor the actor should be registered with provider "explicit-provider"
And v3actor the actor should be registered with model "gpt-4"
Scenario: Nested actors map with no provider or model still raises validation error
Given a nested LLM actor YAML with no provider or model fields with name "local/no-provider-nested-llm"
When I run the actor add command for "local/no-provider-nested-llm" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "provider"
Scenario: Explicit config.model takes precedence over model derived from config.actor
Given a nested LLM actor YAML with explicit config.model overriding config.actor with name "local/explicit-model-llm"
When I run the actor add command for "local/explicit-model-llm" with the prepared config file
Then v3actor the command should succeed
And v3actor the actor should be registered with provider "anthropic"
And v3actor the actor should be registered with model "explicit-model"
# ────────────────────────────────────────────────────────────
# Malformed config.actor edge cases
# ────────────────────────────────────────────────────────────
Scenario: Combined config.actor with empty model half is treated as absent
Given a nested LLM actor YAML with combined config.actor "provider/" and name "local/empty-model-actor"
When I run the actor add command for "local/empty-model-actor" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "provider"
Scenario: Combined config.actor with empty provider half is treated as missing provider
Given a nested LLM actor YAML with combined config.actor "/model" and name "local/empty-provider-actor"
When I run the actor add command for "local/empty-provider-actor" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "provider"
Scenario: Combined config.actor without slash delimiter is treated as absent
Given a nested LLM actor YAML with combined config.actor "noslash" and name "local/noslash-actor"
When I run the actor add command for "local/noslash-actor" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "provider"
Scenario: Combined config.actor with both halves empty is treated as absent
Given a nested LLM actor YAML with combined config.actor "/" and name "local/slash-only-actor"
When I run the actor add command for "local/slash-only-actor" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "provider"
@@ -0,0 +1,63 @@
Feature: Agent Evolution Pool Supervisor - Type Label and Milestone Assignment
As an agent evolution supervisor
I want to automatically assign Type/Automation label and milestone to improvement PRs
So that all improvement PRs have required metadata and don't get blocked by compliance issues
Background:
Given the agent-evolution-pool-supervisor is configured
And the repository has the Type/Automation label with ID 1397
And the repository has an open milestone "v3.2.0" with ID 42
Scenario: Supervisor looks up Type/Automation label ID before creating PR
Given the supervisor is about to create an improvement PR
When the supervisor looks up the Type/Automation label
Then the label ID 1397 is found
And the label name is "Type/Automation"
Scenario: Supervisor looks up earliest open milestone before creating PR
Given the supervisor is about to create an improvement PR
And the repository has multiple open milestones
When the supervisor looks up the earliest open milestone
Then the earliest milestone "v3.2.0" with ID 42 is found
And the milestone is in open state
Scenario: Supervisor passes label and milestone to worker
Given the supervisor has identified an improvement proposal
And the Type/Automation label ID is 1397
And the earliest open milestone ID is 42
When the supervisor dispatches a worker to create the PR
Then the worker receives the label ID 1397 in the prompt
And the worker receives the milestone ID 42 in the prompt
Scenario: Worker creates PR with Type/Automation label and milestone
Given a worker is creating an improvement PR
And the worker has label ID 1397 for Type/Automation
And the worker has milestone ID 42 for v3.2.0
When the worker creates the PR using pr-creator
Then the PR is created with Type/Automation label
And the PR is assigned to milestone v3.2.0
Scenario: Supervisor handles missing Type/Automation label gracefully
Given the supervisor is about to create an improvement PR
And the Type/Automation label does not exist in the repository
When the supervisor looks up the Type/Automation label
Then the label lookup returns no result
And the supervisor logs a warning about missing label
And the supervisor continues without assigning a label
Scenario: Supervisor handles no open milestones gracefully
Given the supervisor is about to create an improvement PR
And there are no open milestones in the repository
When the supervisor looks up the earliest open milestone
Then the milestone lookup returns no result
And the supervisor logs a warning about missing milestones
And the supervisor continues without assigning a milestone
Scenario: Agent definition documents label and milestone lookup steps
Given the agent-evolution-pool-supervisor.md file exists
When I read the agent definition
Then the definition includes a section for label lookup
And the definition includes a section for milestone lookup
And the definition explains how to pass these to the worker
And the definition includes error handling for missing label or milestone
@@ -0,0 +1,58 @@
@mock_only
Feature: PR Compliance Checklist in Implementation Pool Supervisor
As a pool supervisor
I want to pass a mandatory PR compliance checklist to every worker prompt
So that implementation workers complete all required items before creating a PR and avoid systemic merge blockers
Background:
Given the implementation-pool-supervisor.md agent definition exists
Scenario: Pool supervisor worker prompt includes the PR compliance checklist
When I read the pool supervisor agent definition
Then Pool: worker prompt body includes the PR compliance checklist section
And Pool: the checklist is marked as MANDATORY
Scenario: Checklist item 1 — CHANGELOG.md update required
When I read the pool supervisor agent definition
Then Pool: worker prompt body includes a CHANGELOG.md checklist item
And Pool: the item instructs workers to add an entry under the Unreleased section
Scenario: Checklist item 2 — CONTRIBUTORS.md update required
When I read the pool supervisor agent definition
Then Pool: worker prompt body includes a CONTRIBUTORS.md checklist item
And Pool: the item instructs workers to add or update their contribution entry
Scenario: Checklist item 3 — commit footer required
When I read the pool supervisor agent definition
Then Pool: worker prompt body includes a commit footer checklist item
And Pool: the item specifies the ISSUES CLOSED footer format
Scenario: Checklist item 4 — CI must pass before PR creation
When I read the pool supervisor agent definition
Then Pool: worker prompt body includes a CI passes checklist item
And Pool: the item instructs workers to verify all quality gates are green
Scenario: Checklist item 5 — BDD/Behave tests required
When I read the pool supervisor agent definition
Then Pool: worker prompt body includes a BDD tests checklist item
And Pool: the item instructs workers to add or update Behave feature files
Scenario: Checklist item 6 — Epic reference required in PR description
When I read the pool supervisor agent definition
Then Pool: worker prompt body includes an Epic reference checklist item
And Pool: the item instructs workers to reference the parent Epic issue number
Scenario: Checklist item 7 — Labels must be applied
When I read the pool supervisor agent definition
Then Pool: worker prompt body includes a labels checklist item
And Pool: the item instructs workers to apply labels via forgejo-label-manager
Scenario: Checklist item 8 — Milestone must be assigned
When I read the pool supervisor agent definition
Then Pool: worker prompt body includes a milestone checklist item
And Pool: the item instructs workers to assign the earliest open milestone
Scenario: All 8 checklist items are present in the worker prompt
When I read the pool supervisor agent definition
Then Pool: worker prompt body contains all 8 mandatory checklist items
@@ -440,22 +440,15 @@ def step_run_actor_add(context: Context, name: str) -> None:
assert result.exception is None, f"Unexpected exception: {result.exception}"
context.command_error = None
context.actor_registered = True
# Verify registry was called and extract actor metadata from the config blob
# Verify registry was called and extract the actor_type from the config
if mock_registry.upsert_actor.called:
call_kwargs = mock_registry.upsert_actor.call_args
blob = call_kwargs.kwargs.get("config_blob") if call_kwargs.kwargs else None
if isinstance(blob, dict):
context.registered_actor_type = blob.get("type")
context.registered_actor_provider = blob.get("provider")
context.registered_actor_model = blob.get("model")
else:
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
context.registered_actor_type = (
blob.get("type") if isinstance(blob, dict) else None
)
else:
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
else:
assert isinstance(result.exception, SystemExit), (
f"Expected SystemExit, got: {result.exception}"
@@ -463,8 +456,6 @@ def step_run_actor_add(context: Context, name: str) -> None:
context.command_error = result.output
context.actor_registered = False
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
@when('I run the actor update command for "{name}" with the prepared config file')
@@ -541,22 +532,15 @@ def step_run_actor_update(context: Context, name: str) -> None:
assert result.exception is None, f"Unexpected exception: {result.exception}"
context.command_error = None
context.actor_registered = True
# Verify registry was called and extract actor metadata from the config blob
# Verify registry was called and extract the actor_type from the config
if mock_registry.upsert_actor.called:
call_kwargs = mock_registry.upsert_actor.call_args
blob = call_kwargs.kwargs.get("config_blob") if call_kwargs.kwargs else None
if isinstance(blob, dict):
context.registered_actor_type = blob.get("type")
context.registered_actor_provider = blob.get("provider")
context.registered_actor_model = blob.get("model")
else:
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
context.registered_actor_type = (
blob.get("type") if isinstance(blob, dict) else None
)
else:
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
else:
assert isinstance(result.exception, SystemExit), (
f"Expected SystemExit, got: {result.exception}"
@@ -564,8 +548,6 @@ def step_run_actor_update(context: Context, name: str) -> None:
context.command_error = result.output
context.actor_registered = False
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
# ── Then Steps (Assertions) — prefixed with "v3actor" to avoid conflicts ─────
@@ -1145,206 +1127,6 @@ route:
context.config_file = _write_yaml_file(context, "bad-edge-ref", yaml_content)
# ── Nested actors map: combined config.actor field (bug #11189) ───────────────
@given('a nested LLM actor YAML using combined config.actor field with name "{name}"')
def step_nested_llm_combined_actor(context: Context, name: str) -> None:
"""Create a spec-compliant nested-actors-map YAML using the combined config.actor field.
The ``actors:`` map format places ``type: llm`` at the actor-entry level
(``actors.<name>.type``), not inside the ``config:`` block. The provider
and model are supplied via the combined ``config.actor: provider/model``
shorthand rather than separate ``config.provider``/``config.model`` keys.
This is the exact format described in the spec and reproduced in bug #11189.
"""
yaml_content = f"""\
name: {name}
cleveragents:
version: "3.0"
default_actor: main
actors:
main:
type: llm
config:
actor: anthropic/claude-sonnet-4-5
system_prompt: "You are a test actor."
temperature: 0.3
"""
context.config_file = _write_yaml_file(context, name, yaml_content)
@given(
"a nested LLM actor YAML with explicit config.provider overriding config.actor"
' with name "{name}"'
)
def step_nested_llm_explicit_provider_overrides_combined(
context: Context, name: str
) -> None:
"""Create a nested-actors-map YAML with both config.provider and config.actor set.
When ``config.provider`` is explicitly present it must take precedence over
the provider parsed from the combined ``config.actor: provider/model``
shorthand. The model half of the combined field is still used as the
model fallback (since no separate ``config.model`` is provided here).
"""
yaml_content = f"""\
name: {name}
cleveragents:
version: "3.0"
default_actor: main
actors:
main:
type: llm
config:
provider: explicit-provider
actor: override/gpt-4
system_prompt: "Precedence test actor."
"""
context.config_file = _write_yaml_file(context, name, yaml_content)
@given('a nested LLM actor YAML with no provider or model fields with name "{name}"')
def step_nested_llm_no_provider_or_model(context: Context, name: str) -> None:
"""Create a nested-actors-map YAML with type: llm but no provider or model.
When neither ``config.provider``, ``config.model``, nor ``config.actor``
are present the extraction must fail gracefully and propagate a validation
error message so that users receive a clear diagnostic.
"""
yaml_content = f"""\
name: {name}
cleveragents:
version: "3.0"
default_actor: main
actors:
main:
type: llm
config:
system_prompt: "Actor without provider or model."
"""
context.config_file = _write_yaml_file(context, name, yaml_content)
@given(
"a nested LLM actor YAML with explicit config.model overriding config.actor"
' with name "{name}"'
)
def step_nested_llm_explicit_model_overrides_combined(
context: Context, name: str
) -> None:
"""Create a nested-actors-map YAML with both config.model and config.actor set.
When ``config.model`` is explicitly present it must take precedence over
the model parsed from the combined ``config.actor: provider/model``
shorthand. The provider half of the combined field is still used as the
provider fallback (since no separate ``config.provider`` is provided here).
"""
yaml_content = f"""\
name: {name}
cleveragents:
version: "3.0"
default_actor: main
actors:
main:
type: llm
config:
model: explicit-model
actor: anthropic/claude-sonnet-4-5
system_prompt: "Model precedence test actor."
"""
context.config_file = _write_yaml_file(context, name, yaml_content)
@given(
'a nested LLM actor YAML with combined config.actor "{combined_value}"'
' and name "{name}"'
)
def step_nested_llm_malformed_combined_actor(
context: Context, combined_value: str, name: str
) -> None:
"""Create a nested-actors-map YAML with a specific config.actor value.
Used to test edge cases of the combined ``config.actor: provider/model``
shorthand: empty model half (``provider/``), empty provider half
(``/model``), no delimiter (``noslash``), empty string, etc.
"""
yaml_content = f"""\
name: {name}
cleveragents:
version: "3.0"
default_actor: main
actors:
main:
type: llm
config:
actor: {combined_value}
system_prompt: "Malformed actor test."
"""
context.config_file = _write_yaml_file(context, name, yaml_content)
@then(
'v3actor the actor should be registered with provider "{provider}" and model "{model}"'
)
def step_actor_registered_with_provider_and_model(
context: Context, provider: str, model: str
) -> None:
"""Assert the actor was registered with the expected provider and model.
Inspects ``registered_actor_provider`` and ``registered_actor_model``
captured by the ``@when`` step from the ``config_blob`` passed to
``registry.upsert_actor()`` after CLI canonicalization. Canonicalization
calls ``setdefault`` so the canonical blob always carries the resolved
provider and model even when they were not present in the original YAML.
"""
assert context.actor_registered, (
"Actor was not registered. Output: "
f"{context.command_result.get('output', '') if context.command_result else ''}"
)
reg_provider = getattr(context, "registered_actor_provider", None)
reg_model = getattr(context, "registered_actor_model", None)
assert reg_provider == provider, (
f"Expected provider '{provider}' but got '{reg_provider}'"
)
assert reg_model == model, f"Expected model '{model}' but got '{reg_model}'"
@then('v3actor the actor should be registered with provider "{provider}"')
def step_actor_registered_with_provider(context: Context, provider: str) -> None:
"""Assert the actor was registered with the expected provider.
Checks ``registered_actor_provider`` captured by the ``@when`` step from
the ``config_blob`` passed to ``registry.upsert_actor()`` after
CLI canonicalization.
"""
assert context.actor_registered, (
"Actor was not registered. Output: "
f"{context.command_result.get('output', '') if context.command_result else ''}"
)
reg_provider = getattr(context, "registered_actor_provider", None)
assert reg_provider == provider, (
f"Expected provider '{provider}' but got '{reg_provider}'"
)
@then('v3actor the actor should be registered with model "{model}"')
def step_actor_registered_with_model(context: Context, model: str) -> None:
"""Assert the actor was registered with the expected model.
Checks ``registered_actor_model`` captured by the ``@when`` step from
the ``config_blob`` passed to ``registry.upsert_actor()`` after
CLI canonicalization.
"""
assert context.actor_registered, (
"Actor was not registered. Output: "
f"{context.command_result.get('output', '') if context.command_result else ''}"
)
reg_model = getattr(context, "registered_actor_model", None)
assert reg_model == model, f"Expected model '{model}' but got '{reg_model}'"
# ── Helper Functions ──────────────────────────────────────────────────────────
@@ -0,0 +1,382 @@
"""Step definitions for agent evolution pool supervisor metadata assignment."""
from pathlib import Path
from typing import Any
from behave import given, then, when
@given("the agent-evolution-pool-supervisor is configured")
def step_supervisor_configured(context: Any) -> None:
"""Initialize the supervisor context."""
context.supervisor_config = {
"repo_owner": "cleveragents",
"repo_name": "cleveragents-core",
"forgejo_url": "https://git.cleverthis.com",
}
context.labels = {}
context.milestones = {}
@given("the repository has the Type/Automation label with ID {label_id:d}")
def step_repo_has_type_automation_label(context: Any, label_id: int) -> None:
"""Set up the Type/Automation label in the repository."""
context.labels["Type/Automation"] = label_id
@given(
'the repository has an open milestone "{milestone_name}" with ID {milestone_id:d}'
)
def step_repo_has_open_milestone(
context: Any, milestone_name: str, milestone_id: int
) -> None:
"""Set up an open milestone in the repository."""
context.milestones[milestone_name] = {
"id": milestone_id,
"state": "open",
"name": milestone_name,
}
@given("the supervisor is about to create an improvement PR")
def step_supervisor_about_to_create_pr(context: Any) -> None:
"""Set up the context for PR creation."""
context.pr_creation_context = {
"proposal_issue": 7888,
"branch": "improve/agent-evolution-pool-supervisor-metadata",
"title": "Proposal: improve agent-evolution-pool-supervisor — add Type label "
"and milestone assignment to improvement PRs",
}
@when("the supervisor looks up the Type/Automation label")
def step_supervisor_looks_up_label(context: Any) -> None:
"""Simulate looking up the Type/Automation label."""
label_name = "Type/Automation"
if label_name in context.labels:
context.found_label = {
"name": label_name,
"id": context.labels[label_name],
}
else:
context.found_label = None
@then("the label ID {label_id:d} is found")
def step_label_id_found(context: Any, label_id: int) -> None:
"""Verify the label ID was found."""
assert context.found_label is not None, "Label should be found"
assert context.found_label["id"] == label_id, (
f"Expected label ID {label_id}, got {context.found_label['id']}"
)
@then('the label name is "{label_name}"')
def step_label_name_is(context: Any, label_name: str) -> None:
"""Verify the label name."""
assert context.found_label is not None, "Label should be found"
assert context.found_label["name"] == label_name, (
f"Expected label name {label_name}, got {context.found_label['name']}"
)
@given("the repository has multiple open milestones")
def step_repo_has_multiple_milestones(context: Any) -> None:
"""Add multiple open milestones to the repository."""
context.milestones["v3.2.0"] = {
"id": 42,
"state": "open",
"name": "v3.2.0",
"due_on": "2026-01-31T23:59:59Z",
}
context.milestones["v3.3.0"] = {
"id": 43,
"state": "open",
"name": "v3.3.0",
"due_on": "2026-02-28T23:59:59Z",
}
context.milestones["v3.4.0"] = {
"id": 44,
"state": "open",
"name": "v3.4.0",
"due_on": "2026-03-31T23:59:59Z",
}
@when("the supervisor looks up the earliest open milestone")
def step_supervisor_looks_up_earliest_milestone(context: Any) -> None:
"""Simulate looking up the earliest open milestone."""
open_milestones = [m for m in context.milestones.values() if m["state"] == "open"]
if open_milestones:
# Sort by due_on date if available, otherwise by name
sorted_milestones = sorted(
open_milestones,
key=lambda m: m.get("due_on", m["name"]),
)
context.found_milestone = sorted_milestones[0]
else:
context.found_milestone = None
@then('the earliest milestone "{milestone_name}" with ID {milestone_id:d} is found')
def step_earliest_milestone_found(
context: Any, milestone_name: str, milestone_id: int
) -> None:
"""Verify the earliest milestone was found."""
assert context.found_milestone is not None, "Milestone should be found"
assert context.found_milestone["name"] == milestone_name, (
f"Expected milestone {milestone_name}, got {context.found_milestone['name']}"
)
assert context.found_milestone["id"] == milestone_id, (
f"Expected milestone ID {milestone_id}, got {context.found_milestone['id']}"
)
@then("the milestone is in open state")
def step_milestone_is_open(context: Any) -> None:
"""Verify the milestone is in open state."""
assert context.found_milestone is not None, "Milestone should be found"
assert context.found_milestone["state"] == "open", (
f"Expected milestone state 'open', got {context.found_milestone['state']}"
)
@given("the supervisor has identified an improvement proposal")
def step_supervisor_identified_proposal(context: Any) -> None:
"""Set up the proposal context."""
context.proposal = {
"issue_number": 7888,
"title": "Proposal: improve agent-evolution-pool-supervisor — add Type label "
"and milestone assignment to improvement PRs",
"description": "Add Type/Automation label and milestone assignment "
"to improvement PRs",
}
@given("the Type/Automation label ID is {label_id:d}")
def step_label_id_is(context: Any, label_id: int) -> None:
"""Set the label ID in the context."""
context.label_id_for_pr = label_id
@given("the earliest open milestone ID is {milestone_id:d}")
def step_milestone_id_is(context: Any, milestone_id: int) -> None:
"""Set the milestone ID in the context."""
context.milestone_id_for_pr = milestone_id
@when("the supervisor dispatches a worker to create the PR")
def step_supervisor_dispatches_worker(context: Any) -> None:
"""Simulate dispatching a worker with the metadata."""
context.worker_prompt = {
"proposal": context.proposal,
"label_id": context.label_id_for_pr,
"milestone_id": context.milestone_id_for_pr,
"branch": "improve/agent-evolution-pool-supervisor-metadata",
}
@then("the worker receives the label ID {label_id:d} in the prompt")
def step_worker_receives_label_id(context: Any, label_id: int) -> None:
"""Verify the worker receives the label ID."""
assert context.worker_prompt is not None, "Worker prompt should be set"
assert context.worker_prompt["label_id"] == label_id, (
f"Expected label ID {label_id}, got {context.worker_prompt['label_id']}"
)
@then("the worker receives the milestone ID {milestone_id:d} in the prompt")
def step_worker_receives_milestone_id(context: Any, milestone_id: int) -> None:
"""Verify the worker receives the milestone ID."""
assert context.worker_prompt is not None, "Worker prompt should be set"
assert context.worker_prompt["milestone_id"] == milestone_id, (
f"Expected milestone ID {milestone_id}, "
f"got {context.worker_prompt['milestone_id']}"
)
@given("a worker is creating an improvement PR")
def step_worker_creating_pr(context: Any) -> None:
"""Set up the worker context."""
context.worker_context = {
"branch": "improve/agent-evolution-pool-supervisor-metadata",
"title": "Proposal: improve agent-evolution-pool-supervisor — add Type label "
"and milestone assignment to improvement PRs",
}
@given("the worker has label ID {label_id:d} for Type/Automation")
def step_worker_has_label_id(context: Any, label_id: int) -> None:
"""Set the label ID for the worker."""
context.worker_context["label_id"] = label_id
@given("the worker has milestone ID {milestone_id:d} for v3.2.0")
def step_worker_has_milestone_id(context: Any, milestone_id: int) -> None:
"""Set the milestone ID for the worker."""
context.worker_context["milestone_id"] = milestone_id
@when("the worker creates the PR using pr-creator")
def step_worker_creates_pr(context: Any) -> None:
"""Simulate the worker creating a PR with metadata."""
context.created_pr = {
"branch": context.worker_context["branch"],
"title": context.worker_context["title"],
"labels": [context.worker_context["label_id"]],
"milestone": context.worker_context["milestone_id"],
}
@then("the PR is created with Type/Automation label")
def step_pr_has_type_automation_label(context: Any) -> None:
"""Verify the PR has the Type/Automation label."""
assert context.created_pr is not None, "PR should be created"
assert 1397 in context.created_pr["labels"], (
f"Expected label ID 1397 in {context.created_pr['labels']}"
)
@then("the PR is assigned to milestone v3.2.0")
def step_pr_assigned_to_milestone(context: Any) -> None:
"""Verify the PR is assigned to the milestone."""
assert context.created_pr is not None, "PR should be created"
assert context.created_pr["milestone"] == 42, (
f"Expected milestone ID 42, got {context.created_pr['milestone']}"
)
@given("the Type/Automation label does not exist in the repository")
def step_label_does_not_exist(context: Any) -> None:
"""Remove the Type/Automation label from the repository."""
if "Type/Automation" in context.labels:
del context.labels["Type/Automation"]
@then("the label lookup returns no result")
def step_label_lookup_returns_no_result(context: Any) -> None:
"""Verify the label lookup returns no result."""
assert context.found_label is None, "Label lookup should return None"
@then("the supervisor logs a warning about missing label")
def step_supervisor_logs_warning_label(context: Any) -> None:
"""Verify the supervisor records a warning when the label is missing."""
assert context.found_label is None, (
"Label should be None when logging a missing-label warning"
)
context.warnings = getattr(context, "warnings", [])
context.warnings.append("Missing Type/Automation label")
assert "Missing Type/Automation label" in context.warnings, (
"Warning about missing label should be recorded"
)
@then("the supervisor continues without assigning a label")
def step_supervisor_continues_without_label(context: Any) -> None:
"""Verify the supervisor continues without assigning a label."""
assert not hasattr(context, "label_id_for_pr") or context.label_id_for_pr is None, (
"Label ID should not be assigned when label is missing"
)
@given("there are no open milestones in the repository")
def step_no_open_milestones(context: Any) -> None:
"""Remove all open milestones from the repository."""
context.milestones = {
k: v for k, v in context.milestones.items() if v["state"] != "open"
}
@then("the milestone lookup returns no result")
def step_milestone_lookup_returns_no_result(context: Any) -> None:
"""Verify the milestone lookup returns no result."""
assert context.found_milestone is None, "Milestone lookup should return None"
@then("the supervisor logs a warning about missing milestones")
def step_supervisor_logs_warning_milestone(context: Any) -> None:
"""Verify the supervisor records a warning when milestones are missing."""
assert context.found_milestone is None, (
"Milestone should be None when logging a missing-milestone warning"
)
context.warnings = getattr(context, "warnings", [])
context.warnings.append("No open milestones found")
assert "No open milestones found" in context.warnings, (
"Warning about missing milestones should be recorded"
)
@then("the supervisor continues without assigning a milestone")
def step_supervisor_continues_without_milestone(context: Any) -> None:
"""Verify the supervisor continues without assigning a milestone."""
assert (
not hasattr(context, "milestone_id_for_pr")
or context.milestone_id_for_pr is None
), "Milestone ID should not be assigned when milestone is missing"
@given("the agent-evolution-pool-supervisor.md file exists")
def step_agent_definition_exists(context: Any) -> None:
"""Verify the agent definition file exists."""
possible_paths = [
Path(".opencode/agents/agent-evolution-pool-supervisor.md"),
Path("/app/.opencode/agents/agent-evolution-pool-supervisor.md"),
]
agent_file = None
for path in possible_paths:
if path.exists():
agent_file = path
break
assert agent_file is not None, (
f"Agent definition file should exist at one of: {possible_paths}"
)
context.agent_file_path = agent_file
@when("I read the agent definition")
def step_read_agent_definition(context: Any) -> None:
"""Read the agent definition file."""
with open(context.agent_file_path) as f:
context.agent_definition = f.read()
@then("the definition includes a section for label lookup")
def step_definition_includes_label_lookup(context: Any) -> None:
"""Verify the definition includes label lookup documentation."""
assert "label" in context.agent_definition.lower(), (
"Definition should mention label lookup"
)
assert (
"Type/Automation" in context.agent_definition
or "type/automation" in context.agent_definition.lower()
), "Definition should mention Type/Automation label"
@then("the definition includes a section for milestone lookup")
def step_definition_includes_milestone_lookup(context: Any) -> None:
"""Verify the definition includes milestone lookup documentation."""
assert "milestone" in context.agent_definition.lower(), (
"Definition should mention milestone lookup"
)
@then("the definition explains how to pass these to the worker")
def step_definition_explains_passing_to_worker(context: Any) -> None:
"""Verify the definition explains how to pass metadata to the worker."""
assert "worker" in context.agent_definition.lower(), (
"Definition should mention passing to worker"
)
@then("the definition includes error handling for missing label or milestone")
def step_definition_includes_error_handling(context: Any) -> None:
"""Verify the definition includes error handling."""
definition_lower = context.agent_definition.lower()
assert (
"error" in definition_lower
or "handle" in definition_lower
or "gracefully" in definition_lower
), "Definition should include error handling"
@@ -0,0 +1,219 @@
"""Step definitions for PR compliance checklist in implementation pool supervisor.
This file uses parameterized @then decorators with unique step text that
distinguishes pool-supervisor checks from the shared compliance checklist
steps in pr_compliance_checklist_steps.py, preventing Behave AmbiguousStep
errors when both feature files are run together.
Each validator is imported from the shared pr_compliance_checklist_steps module's
validation logic (via the _verify module) to avoid code duplication while using
unique step text prefixes ("Pool:") for disambiguation.
"""
from collections.abc import Callable
from pathlib import Path
from typing import Any
from behave import given, then, when
PROJECT_ROOT = Path(__file__).resolve().parents[2]
AGENT_DEF_PATH = (
PROJECT_ROOT / ".opencode" / "agents" / "implementation-pool-supervisor.md"
)
# ---------------------------------------------------------------------------
# Shared validation helpers — identical logic to pr_compliance_checklist_steps.py
# ---------------------------------------------------------------------------
_required_items = [
"CHANGELOG.md",
"CONTRIBUTORS.md",
"ISSUES CLOSED",
"CI passes",
"BDD/Behave tests",
"Epic reference",
"forgejo-label-manager",
"earliest open milestone",
]
VALIDATORS: dict[str, Callable[[str], bool]] = {
"includes checklist section": lambda c: "PR Compliance Checklist" in c,
"is marked MANDATORY": lambda c: "MANDATORY" in c,
"has CHANGELOG.md item": lambda c: "CHANGELOG.md" in c,
"references Unreleased": lambda c: "[Unreleased]" in c,
"has CONTRIBUTORS.md item": lambda c: "CONTRIBUTORS.md" in c,
"instructs add or update": lambda c: "add or update" in c,
"has commit footer item": lambda c: "Commit footer" in c,
"specifies ISSUES CLOSED": lambda c: "ISSUES CLOSED" in c,
"has CI passes item": lambda c: "CI passes" in c,
"mentions quality gates": lambda c: "quality gates" in c,
"has BDD tests item": lambda c: "BDD/Behave tests" in c,
"instructs add or update features": lambda c: "added or updated" in c,
"has Epic reference item": lambda c: "Epic reference" in c,
"references parent Epic": lambda c: "parent Epic" in c,
"has labels item": lambda c: "Labels" in c,
"mentions forgejo-label-manager": lambda c: "forgejo-label-manager" in c,
"has milestone item": lambda c: "Milestone" in c,
"earliest open milestone": lambda c: "earliest open milestone" in c,
"all 8 items present": lambda c: all(item in c for item in _required_items),
}
def _make_validator(key: str) -> Callable[[Any], None]:
"""Factory that creates a typed Behave validator from a shared helper."""
def validator(context: Any) -> None:
content = context.agent_def_content
check_fn = VALIDATORS.get(key)
assert check_fn(content), f"Pool supervisor agent definition failed: {key}"
return validator
# ---------------------------------------------------------------------------
# Unique @given and @when — scoped to the pool supervisor agent def only
# ---------------------------------------------------------------------------
@given("the implementation-pool-supervisor.md agent definition exists")
def step_agent_def_exists(context: Any) -> None:
"""Verify the pool supervisor agent definition file exists."""
assert AGENT_DEF_PATH.exists(), f"Agent definition not found at {AGENT_DEF_PATH}"
context.agent_def_path = AGENT_DEF_PATH
@when("I read the pool supervisor agent definition")
def step_read_agent_def(context: Any) -> None:
"""Read the pool supervisor agent definition."""
context.agent_def_content = AGENT_DEF_PATH.read_text(encoding="utf-8")
# ---------------------------------------------------------------------------
# Unique @then — prefixed with "Pool:" so they never conflict with the
# shared pr_compliance_checklist_steps.py step definitions.
# ---------------------------------------------------------------------------
# Scenario: Pool supervisor worker prompt includes the PR compliance checklist
@then("Pool: worker prompt body includes the PR compliance checklist section")
def pool_step_prompt_includes_checklist(context: Any) -> None:
"""Verify the pool-supervisor worker prompt body includes the PR compliance checklist."""
_make_validator("includes checklist section")(context)
@then("Pool: the checklist is marked as MANDATORY")
def pool_step_checklist_is_mandatory(context: Any) -> None:
"""Verify the pool-supervisor checklist is marked as MANDATORY."""
_make_validator("is marked MANDATORY")(context)
# Scenario: Checklist item 1 — CHANGELOG.md update required
@then("Pool: worker prompt body includes a CHANGELOG.md checklist item")
def pool_step_prompt_includes_changelog_item(context: Any) -> None:
"""Verify the pool-supervisor worker prompt body includes a CHANGELOG.md checklist item."""
_make_validator("has CHANGELOG.md item")(context)
@then("Pool: the item instructs workers to add an entry under the Unreleased section")
def pool_step_changelog_item_unreleased(context: Any) -> None:
"""Verify the CHANGELOG.md item mentions the Unreleased section."""
_make_validator("references Unreleased")(context)
# Scenario: Checklist item 2 — CONTRIBUTORS.md update required
@then("Pool: worker prompt body includes a CONTRIBUTORS.md checklist item")
def pool_step_prompt_includes_contributors_item(context: Any) -> None:
"""Verify the pool-supervisor worker prompt body includes a CONTRIBUTORS.md checklist item."""
_make_validator("has CONTRIBUTORS.md item")(context)
@then("Pool: the item instructs workers to add or update their contribution entry")
def pool_step_contributors_item_add_update(context: Any) -> None:
"""Verify the CONTRIBUTORS.md item instructs workers to add or update."""
_make_validator("instructs add or update")(context)
# Scenario: Checklist item 3 — commit footer required
@then("Pool: worker prompt body includes a commit footer checklist item")
def pool_step_prompt_includes_commit_footer_item(context: Any) -> None:
"""Verify the pool-supervisor worker prompt body includes a commit footer checklist item."""
_make_validator("has commit footer item")(context)
@then("Pool: the item specifies the ISSUES CLOSED footer format")
def pool_step_commit_footer_issues_closed(context: Any) -> None:
"""Verify the commit footer item specifies the ISSUES CLOSED format."""
_make_validator("specifies ISSUES CLOSED")(context)
# Scenario: Checklist item 4 — CI must pass before PR creation
@then("Pool: worker prompt body includes a CI passes checklist item")
def pool_step_prompt_includes_ci_item(context: Any) -> None:
"""Verify the pool-supervisor worker prompt body includes a CI passes checklist item."""
_make_validator("has CI passes item")(context)
@then("Pool: the item instructs workers to verify all quality gates are green")
def pool_step_ci_item_quality_gates(context: Any) -> None:
"""Verify the CI item instructs workers to verify quality gates are green."""
_make_validator("mentions quality gates")(context)
# Scenario: Checklist item 5 — BDD/Behave tests required
@then("Pool: worker prompt body includes a BDD tests checklist item")
def pool_step_prompt_includes_bdd_item(context: Any) -> None:
"""Verify the pool-supervisor worker prompt body includes a BDD/Behave tests checklist item."""
_make_validator("has BDD tests item")(context)
@then("Pool: the item instructs workers to add or update Behave feature files")
def pool_step_bdd_item_feature_files(context: Any) -> None:
"""Verify the BDD item instructs workers to add or update feature files."""
_make_validator("instructs add or update features")(context)
# Scenario: Checklist item 6 — Epic reference required in PR description
@then("Pool: worker prompt body includes an Epic reference checklist item")
def pool_step_prompt_includes_epic_item(context: Any) -> None:
"""Verify the pool-supervisor worker prompt body includes an Epic reference checklist item."""
_make_validator("has Epic reference item")(context)
@then("Pool: the item instructs workers to reference the parent Epic issue number")
def pool_step_epic_item_parent_reference(context: Any) -> None:
"""Verify the Epic item instructs workers to reference the parent Epic."""
_make_validator("references parent Epic")(context)
# Scenario: Checklist item 7 — Labels must be applied
@then("Pool: worker prompt body includes a labels checklist item")
def pool_step_prompt_includes_labels_item(context: Any) -> None:
"""Verify the pool-supervisor worker prompt body includes a labels checklist item."""
_make_validator("has labels item")(context)
@then("Pool: the item instructs workers to apply labels via forgejo-label-manager")
def pool_step_labels_item_forgejo_label_manager(context: Any) -> None:
"""Verify the labels item instructs workers to use forgejo-label-manager."""
_make_validator("mentions forgejo-label-manager")(context)
# Scenario: Checklist item 8 — Milestone must be assigned
@then("Pool: worker prompt body includes a milestone checklist item")
def pool_step_prompt_includes_milestone_item(context: Any) -> None:
"""Verify the pool-supervisor worker prompt body includes a milestone checklist item."""
_make_validator("has milestone item")(context)
@then("Pool: the item instructs workers to assign the earliest open milestone")
def pool_step_milestone_item_earliest(context: Any) -> None:
"""Verify the milestone item instructs workers to assign the earliest open milestone."""
_make_validator("earliest open milestone")(context)
# Scenario: All 8 checklist items are present in the worker prompt
@then("Pool: worker prompt body contains all 8 mandatory checklist items")
def pool_step_prompt_contains_all_8_items(context: Any) -> None:
"""Verify the pool-supervisor worker prompt body contains all 8 mandatory checklist items."""
_make_validator("all 8 items present")(context)
-44
View File
@@ -488,47 +488,3 @@ def step_persona_bar_reflects_actor(context: object) -> None:
assert "anthropic/claude-4-sonnet" in bar._text, (
f"Expected actor in persona bar, got: {bar._text}"
)
# ---------------------------------------------------------------------------
# TDD regression: issue #11039 - no _render() shadowing Textual Widget._render
# ---------------------------------------------------------------------------
@then("the overlay should not override Textual Widget._render")
def step_overlay_no_render_override(context: object) -> None:
"""Verify that ActorSelectionOverlay does NOT define its own ``_render``.
The Shadowing Bug (issue #11039): ActorSelectionOverlay once defined
``def _render(self) -> None`` which returned ``None``, shadowing
``textual.widgets.Static._render()`` (which returns a
:class:`textual.strip.Strip`). This caused the Textual layout engine to
crash with ``AttributeError: 'NoneType' object has no attribute
'get_height'``.
The fix renamed the method to ``_refresh_display`` so that the inherited
base-class ``_render`` is untouched and returns a proper renderable.
"""
from cleveragents.tui.widgets.actor_selection_overlay import (
ActorSelectionOverlay,
)
cls = ActorSelectionOverlay
# The overlay should NOT define its own _render in its __dict__.
# If it did, _render found on the class would resolve to this method
# rather than the Textual base-class implementation.
has_own_render = "_render" in cls.__dict__
assert not has_own_render, (
"ActorSelectionOverlay defines its own _render() which shadows "
"Textual Widget._render. This is the bug from issue #11039."
)
@then("the overlay _refresh_display method should be callable")
def step_overlay_refresh_display_callable(context: object) -> None:
"""Verify ``_refresh_display`` exists and is callable after show()."""
assert hasattr(context._overlay, "_refresh_display"), (
"Overlay must have _refresh_display method"
)
refresh = context._overlay._refresh_display
assert callable(refresh), "_refresh_display must be callable"
-18
View File
@@ -194,21 +194,3 @@ Feature: TUI first-run experience with actor selection overlay
And I call _complete_first_run with actor "anthropic/claude-4-sonnet"
Then the registry should contain a persona named "default"
And the persona bar should reflect the new actor
# -----------------------------------------------------------------------
# TDD regression: issue #11039 - no _render() shadowing Textual Widget._render
# -----------------------------------------------------------------------
# The overlay formerly defined its own _render() returning None, which
# shadowed textual.widgets.Static._render and caused layout-engine crashes.
# This scenario ensures the rename to _refresh_display eliminated that bug.
@tdd_issue @tdd_issue_11039
Scenario: ActorSelectionOverlay does not define its own _render method
Given a new ActorSelectionOverlay
Then the overlay should not override Textual Widget._render
@tdd_issue @tdd_issue_11039
Scenario: ActorSelectionOverlay has refresh_display callable instead of render
Given a new ActorSelectionOverlay
When I call show on the overlay
Then the overlay _refresh_display method should be callable
@@ -404,37 +404,3 @@ Reject v3 GRAPH Actor With Unreachable Node
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 1
Should Contain ${result.stderr} unreachable
Register LLM Actor With Nested Actors Map And Combined config.actor Field
[Documentation]
... Verify that agents actor add accepts spec-compliant YAML using the
... nested actors map format with type at actor-entry level and provider/model
... supplied via the combined config.actor: "provider/model" shorthand.
... This is the regression test for bug #11189.
[Tags] slow
${yaml_file}= Set Variable ${TEMPDIR}${/}nested_combined_actor.yaml
${yaml_content}= Catenate SEPARATOR=\n
... name: local/nested-combined-llm
... cleveragents:
... ${SPACE}${SPACE}version: "3.0"
... ${SPACE}${SPACE}default_actor: main
... actors:
... ${SPACE}${SPACE}main:
... ${SPACE}${SPACE}${SPACE}${SPACE}type: llm
... ${SPACE}${SPACE}${SPACE}${SPACE}config:
... ${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}actor: anthropic/claude-sonnet-4-5
... ${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}system_prompt: "You are a test actor."
... ${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}temperature: 0.3
Create File ${yaml_file} ${yaml_content}
${result}= Run Process ${PYTHON} ${HELPER} add local/nested-combined-llm ${yaml_file} cwd=${WORKSPACE}
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} actor-add-success
${show_result}= Run Process ${PYTHON} ${HELPER} show local/nested-combined-llm cwd=${WORKSPACE}
Log ${show_result.stdout}
Log ${show_result.stderr}
Should Be Equal As Integers ${show_result.rc} 0
Should Contain ${show_result.stdout} actor-show-success
Should Contain ${show_result.stdout} anthropic
Should Contain ${show_result.stdout} claude-sonnet-4-5
+99 -4
View File
@@ -30,6 +30,9 @@ total_max_workers="${CA_MAX_PARALLEL_WORKERS:-4}"
merge_max_workers=$(awk "BEGIN {print int((${total_max_workers} + 7) / 8)}")
imp_max_workers="${total_max_workers}"
rev_max_workers=$(awk "BEGIN {print int((${total_max_workers} + 1) / 2)}")
# Grooming runs cheap metadata-only work via Forgejo API. Each task is fast
# (a few REST calls), so we don't need many parallel slots — ceil(N/4).
groom_max_workers=$(awk "BEGIN {print int((${total_max_workers} + 3) / 4)}")
# Health check intervals (seconds)
DOOM_CHECK_INTERVAL=900 # 15 minutes
@@ -41,6 +44,27 @@ IDLE_THRESHOLD_MS=180000 # 3 minutes — session considered idle if last_active
# Script paths
SCRIPT_DIR="/app/.opencode/skills/auto-agents-system/scripts"
# Make tsx discoverable on PATH. Running `npx --yes tsx` concurrently with
# a cold or partially-warm npm cache causes a race in `~/.npm/_npx/<hash>/`:
# multiple npm processes simultaneously try to rename `node_modules/esbuild`
# to the same atomic-backup name and fail with
# `ENOTEMPTY: rename '.../esbuild' -> '.../esbuild-7kCFO8Lm'`, crashing
# all but one of them and leaving the cache broken for follow-on calls.
# When tsx is on PATH, npx skips its install/cache logic entirely and just
# execs the existing binary — so the race disappears without changing any
# call sites. We install tsx once into ~/.local/node_modules and prepend
# its bin dir to PATH for the rest of this script's process tree.
TSX_PREFIX="${HOME}/.local"
TSX_BIN_DIR="${TSX_PREFIX}/node_modules/.bin"
if ! [[ -x "${TSX_BIN_DIR}/tsx" ]]; then
printf "[%s] Installing tsx into %s …\n" "$(date +%H:%M:%S)" "$TSX_PREFIX" >&2
mkdir -p "$TSX_PREFIX"
npm install --prefix "$TSX_PREFIX" --silent tsx >/dev/null 2>&1 \
|| { printf "ERROR: failed to install tsx into %s\n" "$TSX_PREFIX" >&2; exit 1; }
fi
export PATH="${TSX_BIN_DIR}:${PATH}"
SESSION_LIST="npx --yes tsx ${SCRIPT_DIR}/session_list.ts"
SESSION_FIND_PREFIX="npx --yes tsx ${SCRIPT_DIR}/session_find_by_prefix.ts"
SESSION_START="npx --yes tsx ${SCRIPT_DIR}/session_start.ts"
@@ -251,6 +275,14 @@ send_message_capture() {
local body="$2"
local out_file="$3"
# Refuse to send empty bodies — the server logs them as
# "No user message found in stream. This should never happen."
if [[ -z "$body" ]]; then
log_warn "send_message_capture: refusing to send empty body to ${sid}"
: > "$out_file" 2>/dev/null || true
return 1
fi
# Use prompt_async — fire-and-forget; server returns 204 immediately.
# The synchronous /message endpoint would block until the agent finishes
# its full response (potentially many minutes for a busy worker).
@@ -448,6 +480,27 @@ build_impl_prompt() {
}'
}
build_groom_prompt() {
local item_json="$1"
local kind="$2"
local work_num="$3"
local work_type; work_type=$([ "$kind" == "ISSUE" ] && echo "issue_groom" || echo "pr_groom")
local compact_item
compact_item=$(echo "$item_json" | jq -c .)
jq -n \
--arg wn "$work_num" \
--arg wt "$work_type" \
--argjson ij "$compact_item" \
'{
issue_number: $wn,
pr_number: $wn,
work_type: $wt,
item_json: $ij
}'
}
build_merge_prompt() {
local pr_json="$1"
local pr_num="$2"
@@ -616,6 +669,13 @@ Evaluate the health of this session."
printf '%s' "$eval_prompt" > "$eval_tmp"
local eval_body
eval_body=$(jq -nc --arg a "worker-health-evaluator" --rawfile t "$eval_tmp" '{agent: $a, parts: [{type: "text", text: $t}]}')
if [[ -z "$eval_body" ]]; then
# jq failed (likely couldn't read eval_tmp) — bail rather than send empty
rm -f "$eval_tmp"
delete_session "$eval_sid"
echo "ok"
return
fi
curl -sf -X POST "${BASE}/session/${eval_sid}/prompt_async" -H "Content-Type: application/json" -d "$eval_body" >/dev/null 2>&1
local eval_deadline=$(( $(date +%s) + 120 ))
while [[ $(date +%s) -lt $eval_deadline ]]; do
@@ -671,7 +731,7 @@ run_health_cycle() {
# 2) Idle worker check — collect all idle SIDs across prefixes, then fetch
# their messages in parallel before processing each one.
local all_prefixes=("AUTO-IMP" "AUTO-MRG" "AUTO-REV")
local all_prefixes=("AUTO-IMP" "AUTO-MRG" "AUTO-REV" "AUTO-GROOM")
local -a idle_sids=()
for prefix in "${all_prefixes[@]}"; do
local sessions_json
@@ -850,7 +910,7 @@ fi
# ── Main orchestration loop ───────────────────────────────────────────────
main_loop() {
log "Starting orchestration loop."
log "Worker pools: merge=${merge_max_workers} imp=${imp_max_workers} rev=${rev_max_workers}"
log "Worker pools: merge=${merge_max_workers} imp=${imp_max_workers} rev=${rev_max_workers} groom=${groom_max_workers}"
while ! $STOP_LOOP; do
N=$((N + 1))
@@ -913,11 +973,28 @@ main_loop() {
"Changes Requested|${SCRIPT_DIR}/list_prs_changes_requested.ts|PR"
"CI Failing|${SCRIPT_DIR}/list_prs_ci_failing.ts|PR"
)
# GROOM mirrors IMP: same priority order (PRs first, issues second), same
# work groups. Re-uses IMP's cached results within the same iteration
# via the shared cache_dir, so grooming pre-fetch is effectively free.
declare -a GROOM_WORK_GROUPS=(
"CI Failing PRs|${SCRIPT_DIR}/list_prs_ci_failing.ts|PR"
"Addressed Changes CI Passing|${SCRIPT_DIR}/list_prs_addressed_changes_ci_passing.ts|PR"
"Addressed Changes CI Failing|${SCRIPT_DIR}/list_prs_addressed_changes_ci_failing.ts|PR"
"Changes Requested|${SCRIPT_DIR}/list_prs_changes_requested.ts|PR"
"CI Missing|${SCRIPT_DIR}/list_prs_missing_ci_checks.ts|PR"
"No Active Review CI Passing|${SCRIPT_DIR}/list_prs_no_active_review_ci_passing.ts|PR"
"No Active Review CI Failing|${SCRIPT_DIR}/list_prs_no_active_review_ci_failing.ts|PR"
"Needs Review Not Stale|${SCRIPT_DIR}/list_prs_needs_review_not_stale.ts|PR"
"Needs Review Stale Clean|${SCRIPT_DIR}/list_prs_needs_review_stale_clean.ts|PR"
"Needs Review Stale Conflicts|${SCRIPT_DIR}/list_prs_needs_review_stale_conflicts.ts|PR"
"Open Issues|${SCRIPT_DIR}/list_issues.ts|ISSUE"
)
declare -a pools=(
"AUTO-IMP|implementation-worker|${imp_max_workers}|IMP_WORK_GROUPS"
"AUTO-MRG|pr-merge-worker|${merge_max_workers}|MRG_WORK_GROUPS"
"AUTO-REV|pr-review-worker|${rev_max_workers}|REV_WORK_GROUPS"
"AUTO-GROOM|grooming-worker|${groom_max_workers}|GROOM_WORK_GROUPS"
)
# Shared cache dir for this iteration — pools reuse each other's results
@@ -1029,7 +1106,7 @@ main_loop() {
elif [[ $ec -ne 0 ]]; then
log_warn "[pool:${prefix}] ${ck}: FAILED (exit ${ec}) — treating as empty"
local snippet
snippet=$(head -n 3 "${out_file}.err" 2>/dev/null | tr '\n' '|' | sed 's/|$//')
snippet=$(head -n 20 "${out_file}.err" 2>/dev/null | tr '\n' '|' | sed 's/|$//')
if [[ -n "$snippet" ]]; then
log_warn " stderr: ${snippet}"
fi
@@ -1144,15 +1221,33 @@ main_loop() {
local tmp_prompt
tmp_prompt=$(mktemp "/tmp/ocb-${worker_tag}-${$}.XXXXXX.txt") || continue
local prompt_text
local prompt_text=""
case "$agent" in
*implementation*) prompt_text=$(build_impl_prompt "$item" "$kind" "$work_num") ;;
*merge*) prompt_text=$(build_merge_prompt "$item" "$work_num") ;;
*review*) prompt_text=$(build_review_prompt "$item" "$work_num") ;;
*grooming*) prompt_text=$(build_groom_prompt "$item" "$kind" "$work_num") ;;
esac
# Skip if the build function returned an empty prompt — sending
# an empty body to prompt_async results in "no user message
# found in stream" errors on the server.
if [[ -z "$prompt_text" ]]; then
log_warn "[pool:${prefix}] built empty prompt for ${kind} ${work_num} (${agent}) — skipping launch"
rm -f "$tmp_prompt"
continue
fi
printf '%s' "$prompt_text" > "$tmp_prompt"
# Belt-and-braces: confirm the file actually has content on disk
# before launching the session.
if [[ ! -s "$tmp_prompt" ]]; then
log_warn "[pool:${prefix}] prompt file empty after write for ${kind} ${work_num} (${agent}) — skipping launch"
rm -f "$tmp_prompt"
continue
fi
$SESSION_START --server "${BASE}" --tag "$worker_tag" --agent "$agent" --prompt-file "$tmp_prompt" --restart \
>/dev/null 2>&1 &
pids+=($!)
+31 -85
View File
@@ -12,41 +12,6 @@ from cleveragents.actor.yaml_loader import load_yaml_text
_V3_ACTOR_TYPES: frozenset[str] = frozenset({"llm", "graph", "tool"})
def _parse_combined_actor_field(data: dict[str, Any]) -> tuple[str | None, str | None]:
"""Extract (provider, model) from the combined ``config.actor`` shorthand.
Looks for ``config.actor: "provider/model"`` inside the first entry of
the nested ``actors:`` map. Returns ``(None, None)`` when the shorthand
is absent or malformed (empty provider or model half, missing delimiter).
Args:
data: Top-level actor YAML blob (may contain an ``actors:`` map).
Returns:
``(provider_value, model_value)`` both ``None`` when the combined
field is not present or cannot be parsed.
"""
actors_val = data.get("actors")
if not isinstance(actors_val, dict) or not actors_val:
return None, None
for _, first_entry in actors_val.items():
if not isinstance(first_entry, dict):
continue
config_block = first_entry.get("config")
if isinstance(config_block, dict):
combined = config_block.get("actor")
if isinstance(combined, str) and "/" in combined and combined.strip():
parts = combined.strip().split("/", 1)
provider = parts[0].strip()
model = parts[1].strip()
if provider and model:
return provider, model
break # Only examine the first dict entry in the actors map.
return None, None
class ActorConfiguration(BaseModel):
"""Canonical actor configuration parsed from user-provided blobs."""
@@ -194,14 +159,12 @@ class ActorConfiguration(BaseModel):
The v3 schema is identified by a top-level ``type`` key whose value
is one of ``llm``, ``graph``, or ``tool`` OR by a top-level
``actors:`` map containing at least one entry whose actor-entry level
``actors:`` map containing at least one entry whose ``config:`` block
carries a ``type`` field with one of those values.
Provider and model may appear at the top level OR nested inside
``actors.<actor-name>.config`` (either as separate ``provider``/``model``
keys, or as a combined ``actor: "provider/model"`` shorthand). Explicit
``config.provider`` and ``config.model`` take precedence over the combined
shorthand when both are present.
``actors.<actor-name>.config``. When found in the nested config,
they take precedence over top-level values.
For ``type: graph`` actors the ``route`` block is wrapped into a
graph descriptor.
@@ -216,16 +179,17 @@ class ActorConfiguration(BaseModel):
if isinstance(actors_val, dict) and actors_val:
for _, first_entry in actors_val.items():
if isinstance(first_entry, dict):
# The v3 spec places 'type' at the actor-entry level
# (sibling of 'config'), not inside the config block.
nested_type = first_entry.get("type")
is_v3_type = (
isinstance(nested_type, str)
and nested_type.lower() in _V3_ACTOR_TYPES
)
if is_v3_type:
actor_type = nested_type
break
config_block = first_entry.get("config")
if isinstance(config_block, dict):
nested_type = config_block.get("type")
is_v3_type = (
isinstance(nested_type, str)
and nested_type.lower() in _V3_ACTOR_TYPES
)
if is_v3_type:
actor_type = nested_type
break
break
if not isinstance(actor_type, str):
return None, None, None, False
@@ -233,16 +197,8 @@ class ActorConfiguration(BaseModel):
provider_value = data.get("provider")
unsafe_flag = bool(data.get("unsafe", False))
# Derive provider/model from nested actors map when top-level fields
# are absent. Separate config.provider / config.model keys take
# precedence over the combined config.actor shorthand.
if (not model_value or not isinstance(model_value, str)) or (
not provider_value or not isinstance(provider_value, str)
):
combined_provider, combined_model = _parse_combined_actor_field(data)
if not isinstance(model_value, str) or not model_value:
# First, check for a separate config.model key in the nested map.
model_from_separate: str | None = None
if not model_value or not isinstance(model_value, str):
if actor_type.lower() != "tool":
actors_val = data.get("actors")
if isinstance(actors_val, dict) and actors_val:
for _, first_entry in actors_val.items():
@@ -251,37 +207,27 @@ class ActorConfiguration(BaseModel):
if isinstance(config_block, dict):
mv = config_block.get("model")
if isinstance(mv, str) and mv:
model_from_separate = mv
model_value = mv
break
break
if model_from_separate:
model_value = model_from_separate
elif combined_model:
model_value = combined_model
if not isinstance(provider_value, str) or not provider_value:
# First, check for a separate config.provider key in the nested map.
provider_from_separate: str | None = None
actors_val = data.get("actors")
if isinstance(actors_val, dict) and actors_val:
for _, first_entry in actors_val.items():
if isinstance(first_entry, dict):
config_block = first_entry.get("config")
if isinstance(config_block, dict):
pv = config_block.get("provider")
if isinstance(pv, str) and pv:
provider_from_separate = pv
break
break
if provider_from_separate:
provider_value = provider_from_separate
elif combined_provider:
provider_value = combined_provider
break
if not model_value:
if actor_type.lower() != "tool":
return None, None, None, False
model_value = ""
if not provider_value or not isinstance(provider_value, str):
actors_val = data.get("actors")
if isinstance(actors_val, dict) and actors_val:
for _, first_entry in actors_val.items():
if isinstance(first_entry, dict):
config_block = first_entry.get("config")
if isinstance(config_block, dict):
pv = config_block.get("provider")
if isinstance(pv, str) and pv:
provider_value = pv
break
break
if not provider_value:
provider_value = (
infer_provider_from_model(model_value) if model_value else "custom"
@@ -759,6 +759,14 @@ class ACMSPipeline:
# can wire configuration (e.g. default budget, strategy allow-lists)
# and persistence without changing the constructor signature.
self._settings = settings
# Auto-load strategy configuration from Settings if provided.
# Per spec §42947-42980: when a Settings object is passed to the
# pipeline, its TOML-derived ``context`` configuration is used to
# populate and enable the StrategyRegistry. This eliminates the need
# for callers to manually register every strategy.
if settings is not None:
self._load_strategies_from_settings(settings)
self._unit_of_work = unit_of_work
self._strategies: dict[str, ContextStrategy] = {
@@ -802,6 +810,57 @@ class ACMSPipeline:
self._plugin_manager = plugin_manager
self._discover_context_extensions()
def _load_strategies_from_settings(self, settings: Settings) -> None:
"""Auto-load strategy configuration from a :class:`~cleveragents.config.settings.Settings` object.
Attempts to read ``context.strategies`` configuration from the TOML-based
Settings (typically loaded from a project config file via pydantic-settings).
If present, built-in strategies are registered as per default, custom plugins
from ``module:ClassName`` strings are discovered, and the enabled list is set.
Any errors are logged but do not prevent the pipeline from operating with
its core built-in strategies.
"""
try:
context_cfg = getattr(settings, "context", None)
if context_cfg is None:
return
# Extract the strategies sub-section as a dict.
strategy_cfg: dict[str, Any] = {}
if isinstance(context_cfg, dict):
strat_dict = context_cfg.get("strategies", {}) or {}
if isinstance(strat_dict, dict):
strategy_cfg = dict(strat_dict)
else:
return
# Import the lazy-load helper; avoid circular import at module level.
from cleveragents.application.services.context_strategies import (
load_strategies_from_config,
)
registry = load_strategies_from_config(strategy_cfg)
for name in registry.list_all():
if name not in self._strategies:
strat_inst = registry.get(name)
self.register_strategy(name=name, strategy=strat_inst)
# Sync the enabled list.
enabled = registry.list_enabled()
if enabled:
self._enforce_enabled_strategies(enabled)
except Exception as exc: # noqa: BLE001
self._logger.warning(
"strategy_config_load_failed",
error=str(exc),
)
def _enforce_enabled_strategies(self, enabled_names: list[str]) -> None:
"""Internal helper to filter ``_strategies`` to the enabled subset."""
enabled_set = set(enabled_names)
self._strategies = {
name: strat for name, strat in self._strategies.items() if name in enabled_set
}
# ------------------------------------------------------------------
# Plugin extension point discovery
# ------------------------------------------------------------------
@@ -17,6 +17,11 @@ All strategies implement the v1 ``ContextStrategy`` Protocol defined in
``acms_service.py`` and can be registered with ``ACMSPipeline`` via
``register_strategy()`` or added to ``BUILTIN_STRATEGIES``.
**Registry integration** (spec §47561): This module is the canonical home for
:py:class:`~cleveragents.application.services.strategy_registry.StrategyRegistry`.
The strategies here are registered by default when a registry is created, and
custom strategies can be loaded via :func:`load_strategies_from_config`.
Based on ``docs/specification.md`` §25207-25216.
"""
@@ -36,6 +41,41 @@ from cleveragents.domain.models.core.context_fragment import (
ContextFragment,
)
# ---------------------------------------------------------------------------
# Re-exports for registry integration (spec §47561)
# ---------------------------------------------------------------------------
from cleveragents.application.services.strategy_registry import (
StrategyConfig, # noqa: F401
StrategyNotFoundError, # noqa: F401
StrategyRegistrationError, # noqa: F401
StrategyRegistry, # noqa: F401
StrategyRegistryEntry, # noqa: F401
)
from cleveragents.domain.models.acms.strategy import (
ContextStrategy, # noqa: F401
StrategyCapabilities as _DomainStrategyCapabilities, # noqa: F401
)
__all__: list[str] = [
"SimpleKeywordStrategy",
"SemanticEmbeddingStrategy",
"BreadthDepthNavigatorStrategy",
"StrategyRegistry",
"StrategyConfig",
"StrategyRegistryEntry",
"StrategyNotFoundError",
"StrategyRegistrationError",
]
# Built-in strategy registry constants (used by load_strategies_from_config)
DEFAULT_ENABLED_STRATEGIES: tuple[str, ...] = (
"simple-keyword",
"semantic-embedding",
"breadth-depth-navigator",
)
logger = logging.getLogger(__name__)
_WORD_RE = re.compile(r"\w+", re.UNICODE)
@@ -403,3 +443,59 @@ def _max_proximity(node_uri: str, focus_nodes: list[str], max_hops: int) -> floa
best = max(best, proximity)
return best
# ---------------------------------------------------------------------------
# Helper: load_strategies_from_config (spec §42947-42980)
# ---------------------------------------------------------------------------
def load_strategies_from_config(config: dict[str, Any]) -> StrategyRegistry:
"""Populate and return a :class:`StrategyRegistry` from a TOML config dict.
Accepts a dict representing the parsed ``[context.strategies]`` section of
a TOML configuration file. The expected structure is::
{
"enabled": ["simple-keyword", "semantic-embedding"],
"custom": {
"my-strategy": "my_package.strategies:MyStrategy",
},
}
:param config: Dict with an optional ``"enabled"`` key and an optional
``"custom"`` dict mapping strategy names to ``"module:ClassName"``
strings.
:returns: A fully-populated :class:`StrategyRegistry` ready for use by
an ``ACMSPipeline`` or downstream component.
The built-in strategies from :const:`DEFAULT_ENABLED_STRATEGIES` are
always registered first (as built-ins). Custom strategies are discovered
and loaded via :meth:`StrategyRegistry.register_from_module`. Finally,
the enabled list is set so that only configured strategies are active.
"""
registry = StrategyRegistry()
# Register all built-in strategies from DEFAULT_ENABLED_STRATEGIES
for strategy_class_name in (
"SimpleKeywordStrategy",
"SemanticEmbeddingStrategy",
"BreadthDepthNavigatorStrategy",
):
cls = globals()[strategy_class_name]
instance = cls()
registry.register(
instance,
config=StrategyConfig(),
module_path="cleveragents.application.services.context_strategies",
is_builtin=True,
)
# Register custom strategies from module:ClassName strings
for name, module_path in (config.get("custom") or {}).items():
registry.register_from_module(name=name, module_path=module_path)
# Set the enabled strategy list
enabled = config.get("enabled", DEFAULT_ENABLED_STRATEGIES)
registry.set_enabled(list(enabled))
return registry
@@ -19,6 +19,7 @@ from __future__ import annotations
import os
from collections import defaultdict
from pathlib import Path
# ---------------------------------------------------------------------------
# Token estimation
@@ -74,20 +75,22 @@ def _directory_key(path: str, depth: int = 2, root: str | None = None) -> str:
Returns:
The directory key (first *depth* components of the path).
"""
# Normalize path separators
normalized = path.replace("\\", "/")
# Normalize to pathlib for safe, path-semantic comparisons — never
# string prefix matching (startswith) which is vulnerable to
# /tmp/sandboxmalicious prefix-collision bypass — issue #7478.
norm_path = Path(path)
parts_raw: list[str]
# If root is provided, compute relative path
if root:
root_normalized = root.replace("\\", "/")
# Ensure root ends with / for proper prefix matching
if not root_normalized.endswith("/"):
root_normalized += "/"
# Remove root prefix if path starts with it
if normalized.startswith(root_normalized):
normalized = normalized[len(root_normalized) :]
parts = normalized.split("/")
norm_root = Path(root)
try:
relative = str(norm_path.relative_to(norm_root))
except ValueError:
# path is not under root; use the original path as-is.
relative = path
parts_raw = relative.replace("\\", "/").split("/")
else:
parts_raw = norm_path.parts
# Filter out empty parts (from leading / in absolute paths)
parts = [p for p in parts if p]
@@ -10,6 +10,7 @@ from __future__ import annotations
import os
import re
from pathlib import Path
from datetime import UTC, datetime
from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable
@@ -490,9 +491,13 @@ class LLMExecuteActor:
for match in pattern.finditer(llm_output):
path = match.group(1).strip()
content = match.group(2)
full_path = os.path.normpath(os.path.join(sandbox_root, path))
# Path traversal guard: reject paths escaping sandbox
if not full_path.startswith(sandbox_root + os.sep):
full_path_str = os.path.normpath(os.path.join(sandbox_root, path))
full_path = Path(full_path_str).resolve()
sandbox_resolved = Path(sandbox_root).resolve()
# Path traversal guard: reject paths escaping sandbox using
# path-semantic comparison (never str.startswith) to prevent
# /tmp/sandboxmalicious bypass — see issue #7478.
if not (full_path == sandbox_resolved or full_path.is_relative_to(sandbox_resolved)):
logger.warning(
"Rejected path traversal in LLM output",
path=path,
@@ -23,6 +23,9 @@ from typing import Any
import structlog
from cleveragents.application.services.acms_service import (
StrategyCapabilities as _V1StrategyCapabilities,
)
from cleveragents.domain.models.acms.strategy import (
ContextStrategy,
StrategyConfig,
@@ -32,6 +35,24 @@ from cleveragents.domain.models.acms.strategy import (
logger = structlog.get_logger(__name__)
def _is_v1_pipeline_caps(caps: object) -> bool:
"""Return True if *caps* is a v1-pipeline StrategyCapabilities dataclass.
V1 pipeline strategies (those defined in ``context_strategies.py`` and
``acms_service.py``) use a dataclass-style capabilities object that has
attributes like :attr:`~StrategyCapabilities.supports_semantic_search`,
rather than the domain-model ``StrategyCapabilities`` (from
``strategy.py``) which has :attr:`~StrategyCapabilities.uses_text`,
:attr:`~StrategyCapabilities.resource_types`, etc.
We detect v1-capabilities by checking whether the object is an instance
of the acms_service dataclass. This allows :meth:`validate_registry` to
skip the resource_types check for legacy strategies that do not declare
domain-model backend fields.
"""
return isinstance(caps, _V1StrategyCapabilities)
# ---------------------------------------------------------------------------
# Exceptions
# ---------------------------------------------------------------------------
@@ -489,7 +510,12 @@ class StrategyRegistry:
f"Strategy '{name}' declares no backend capabilities"
)
if not caps.resource_types:
# Skip the resource_types check for v1 pipeline strategies.
# Those use a dataclass-style StrategyCapabilities that lacks
# the domain-model ``resource_types`` field entirely, so all
# warnings about undeclared resource types would be false
# positives. See _is_v1_pipeline_caps().
if not _is_v1_pipeline_caps(caps) and not caps.resource_types:
warnings.append(
f"Strategy '{name}' does not declare supported "
f"resource types (capabilities.resource_types is empty)"
+5 -1
View File
@@ -979,7 +979,11 @@ def _apply_sandbox_changes(
src = os.path.join(dirpath, fname)
rel = os.path.relpath(src, sandbox_root)
dst = os.path.normpath(os.path.join(project_root, rel))
if not dst.startswith(project_root + os.sep):
# Path containment via pathlib to avoid prefix-collision bypass
# (/tmp/sandboxmalicious) — see issue #7478.
_dst_path = Path(dst).resolve()
_pr_path = Path(project_root).resolve()
if not (_dst_path == _pr_path or _dst_path.is_relative_to(_pr_path)):
continue
if rel.split(os.sep)[0] in _skip_dirs:
continue
-17
View File
@@ -112,8 +112,6 @@ class StdioTransport:
cwd=self._cwd,
)
except FileNotFoundError as exc:
self._process = None
from cleveragents.lsp.errors import LspError
raise LspError(
@@ -121,27 +119,12 @@ class StdioTransport:
details={"command": self._command, "args": self._args},
) from exc
except OSError as exc:
# Popen may have partially started the subprocess before
# raising (e.g. execve failure post-fork on some platforms).
# Ensure cleanup so the process does not leak into the caller's
# address space.
if self._process is not None:
self.stop()
self._process = None
from cleveragents.lsp.errors import LspError
raise LspError(
f"Failed to start LSP server: {exc}",
details={"command": self._command, "error": str(exc)},
) from exc
except Exception:
# Catch-all for any other low-level OSError / resource-error
# variants that might leave a zombie process behind.
if self._process is not None:
self.stop()
self._process = None
raise
logger.info(
"lsp.transport.started",
+1 -3
View File
@@ -182,9 +182,7 @@ class BaseResourceHandler:
"""
root = Path(location).resolve()
target = (root / path).resolve()
# Use root + os.sep to prevent prefix collision bypass:
# e.g. root=/tmp/foo must not match target=/tmp/foobar/secret
if target != root and not str(target).startswith(str(root) + os.sep):
if not (target == root or target.is_relative_to(root)):
raise PermissionError(f"Path '{path}' escapes resource root '{location}'")
return target
+5 -5
View File
@@ -73,12 +73,12 @@ def validate_sandbox_path(path_str: str, sandbox_root: str | None = None) -> Pat
if not path_str:
raise ValueError("Path must not be empty")
root = Path(sandbox_root) if sandbox_root else Path.cwd()
root = root.resolve()
root = Path(sandbox_root).resolve() if sandbox_root else Path.cwd().resolve()
target = (root / path_str).resolve()
if not str(target).startswith(str(root)):
raise ValueError(f"Path traversal detected: '{path_str}' escapes sandbox root")
if not (target == root or target.is_relative_to(root)):
raise ValueError(
f"Path traversal detected: '{path_str}' escapes sandbox root '{root}'"
)
return target
+1 -1
View File
@@ -263,7 +263,7 @@ class InlineToolExecutor:
try:
resolved = Path(value).resolve()
sandbox_resolved = sandbox_path.resolve()
if not str(resolved).startswith(str(sandbox_resolved)):
if not (resolved == sandbox_resolved or resolved.is_relative_to(sandbox_resolved)):
return (
f"Path '{value}' for key '{key}' escapes sandbox "
f"root '{sandbox_path}'"
+4 -8
View File
@@ -79,16 +79,12 @@ def validate_path(path_str: str, sandbox_root: str | None = None) -> Path:
Raises ``ValueError`` when the resolved path escapes *sandbox_root*
(defaults to the current working directory when not supplied).
"""
root = Path(sandbox_root) if sandbox_root else Path.cwd()
root = root.resolve()
root = Path(sandbox_root).resolve() if sandbox_root else Path.cwd().resolve()
target = (root / path_str).resolve()
try:
target.relative_to(root)
except ValueError as exc:
if not (target == root or target.is_relative_to(root)):
raise ValueError(
f"Path traversal detected: '{path_str}' escapes sandbox root"
) from exc
f"Path traversal detected: '{path_str}' escapes sandbox root '{root}'"
)
return target
@@ -145,7 +145,7 @@ class ActorSelectionOverlay(_StaticBase):
self._confirmed = False
self._selected_actor = None
self._visible = True
self._refresh_display()
self._render()
def hide(self) -> None:
"""Hide the overlay and clear its content."""
@@ -161,14 +161,14 @@ class ActorSelectionOverlay(_StaticBase):
if not self._filtered_actors:
return
self._selected_index = (self._selected_index - 1) % len(self._filtered_actors)
self._refresh_display()
self._render()
def move_down(self) -> None:
"""Move the selection cursor down by one position (wraps)."""
if not self._filtered_actors:
return
self._selected_index = (self._selected_index + 1) % len(self._filtered_actors)
self._refresh_display()
self._render()
# ------------------------------------------------------------------
# Search / filter
@@ -191,7 +191,7 @@ class ActorSelectionOverlay(_StaticBase):
else:
self._filtered_actors = list(self._actors)
self._selected_index = 0
self._refresh_display()
self._render()
# ------------------------------------------------------------------
# Confirmation
@@ -218,7 +218,7 @@ class ActorSelectionOverlay(_StaticBase):
# Internal rendering
# ------------------------------------------------------------------
def _refresh_display(self) -> None:
def _render(self) -> None:
content = render_actor_selection(
self._filtered_actors,
self._selected_index,
View File
+786
View File
@@ -0,0 +1,786 @@
"""Tests for the ContextStrategyRegistry and plugin loading system.
Covers:
- Strategy registration and retrieval
- Protocol conformance validation
- Plugin loading from ``module:ClassName`` strings
- TOML configuration loading via :func:`load_strategies_from_config`
- Thread safety of registry operations
- Fallback degradation chain verification
- Config-driven strategy enable/disable
"""
from __future__ import annotations
import threading
import time
from collections.abc import Sequence
from typing import Any
import pytest
# ---------------------------------------------------------------------------
# Test fixtures and helpers
# ---------------------------------------------------------------------------
class FakeStrategy:
"""Minimal strategy that satisfies the v1 ``ContextStrategy`` protocol."""
def __init__(self, name: str = "fake") -> None:
self._name = name
@property
def name(self) -> str: # type: ignore[override]
return self._name
@property
def capabilities(self):
from cleveragents.application.services.acms_service import (
StrategyCapabilities as _Caps,
)
return _Caps(supports_semantic_search=False)
def can_handle(self, request: dict[str, Any]) -> float: # type: ignore[override]
return 0.5
def assemble(
self,
fragments: Sequence[Any],
budget: Any,
) -> Sequence[Any]:
return list(fragments)
def explain(self) -> str:
return "Fake strategy for testing."
class FakeDomainStrategy:
"""Strategy that implements the domain-model ``ContextStrategy`` Protocol."""
def __init__(self, name: str = "domain-fake") -> None:
self._name = name
@property
def name(self) -> str: # type: ignore[override]
return self._name
@property
def capabilities(self):
from cleveragents.domain.models.acms.strategy import StrategyCapabilities
return StrategyCapabilities(uses_text=True, resource_types=("text",))
def can_handle(self, request: dict[str, Any], backends: Any) -> float: # type: ignore[override]
return 0.8
def assemble(
self, request: Any, backends: Any, budget: int, plan_context: Any
) -> list: # type: ignore[override]
return []
def explain(self) -> str:
return "Fake domain strategy for testing."
class FakeStrategyWithNoCaps:
"""Strategy with empty capabilities (used to trigger validate_registry warnings)."""
@property
def name(self) -> str:
return "noop-cap-fake"
@property
def capabilities(self):
from cleveragents.domain.models.acms.strategy import StrategyCapabilities
return StrategyCapabilities(uses_text=False, resource_types=()) # empty caps
def can_handle(self, request: dict[str, Any], backends: Any) -> float:
return 0.1
def assemble(self, request: Any, backends: Any, budget: int, plan_context: Any) -> list:
return []
def explain(self) -> str:
return "Strategy with empty capabilities"
# ---------------------------------------------------------------------------
# Tests: Strategy registration and retrieval
# ---------------------------------------------------------------------------
class TestRegistrationAndRetrieval:
"""Validate basic register/get/list operations."""
def test_register_and_get(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
strategy = FakeStrategy("my-strategy")
registry.register(strategy)
assert registry.get("my-strategy") is strategy
assert "my-strategy" in registry
def test_register_with_custom_name(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
strategy = FakeStrategy("original-name")
registry.register(strategy, name="override-name")
assert "override-name" in registry
assert "original-name" not in registry
def test_register_with_config(self):
from cleveragents.application.services.strategy_registry import (
StrategyConfig,
StrategyRegistry,
)
registry = StrategyRegistry()
strategy = FakeStrategy("config-strategy")
cfg = StrategyConfig(enabled=False)
registry.register(strategy, config=cfg)
# With enabled=False the strategy should be registered but not in _enabled_order
entry = registry.get_entry("config-strategy")
assert entry.config.enabled is False
def test_register_builtin_flag(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
strategy = FakeStrategy("built-in-1")
registry.register(strategy, is_builtin=True)
assert "built-in-1" in registry.list_builtin()
entry = registry.get_entry("built-in-1")
assert entry.is_builtin is True
def test_duplicate_registration_raises(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistrationError,
StrategyRegistry,
)
registry = StrategyRegistry()
s = FakeStrategy("dup")
registry.register(s)
with pytest.raises(StrategyRegistrationError):
registry.register(FakeStrategy("dup"))
def test_empty_name_raises(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistrationError,
StrategyRegistry,
)
registry = StrategyRegistry()
class Nameless:
@property
def name(self) -> str: # type: ignore[override]
return ""
@property
def capabilities(self):
from cleveragents.application.services.acms_service import (
StrategyCapabilities as _Caps,
)
return _Caps()
def can_handle(self, request: dict[str, Any]) -> float: # type: ignore[override]
return 0.3
def assemble(
self, fragments: Sequence[Any], budget: Any
) -> Sequence[Any]:
return []
def explain(self) -> str:
return ""
with pytest.raises(StrategyRegistrationError, match="empty"):
registry.register(Nameless())
def test_protocol_violation_raises(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistrationError,
StrategyRegistry,
)
registry = StrategyRegistry()
class NotStrategy: # No name/abilities etc. at all
pass
with pytest.raises(StrategyRegistrationError):
registry.register(NotStrategy()) # type: ignore[arg-type]
def test_list_all(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
for i in range(5):
entry_name = f"strat-{i}"
registry.register(FakeStrategy(entry_name))
all_names = registry.list_all()
assert sorted(all_names) == ["strat-0", "strat-1", "strat-2", "strat-3", "strat-4"]
def test_list_enabled_empty(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
assert registry.list_enabled() == []
def test_get_entry_and_config(self):
from cleveragents.application.services.strategy_registry import (
StrategyConfig,
StrategyNotFoundError,
StrategyRegistry,
)
registry = StrategyRegistry()
cfg = StrategyConfig(timeout_seconds=60)
registry.register(FakeStrategy("cfg-strat"), config=cfg)
entry = registry.get_entry("cfg-strat")
assert entry.name == "cfg-strat"
assert entry.config.timeout_seconds == 60
loaded_cfg = registry.get_config("cfg-strat")
assert loaded_cfg.timeout_seconds == 60
def test_get_raises_on_missing(self):
from cleveragents.application.services.strategy_registry import (
StrategyNotFoundError,
StrategyRegistry,
)
registry = StrategyRegistry()
with pytest.raises(StrategyNotFoundError, match="not found"):
registry.get("nonexistent")
def test_len_and_contains(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
assert len(registry) == 0
assert "empty" not in registry
registry.register(FakeStrategy("one"))
assert len(registry) == 1
assert "one" in registry
def test_unregister(self):
from cleveragents.application.services.strategy_registry import (
StrategyNotFoundError,
StrategyRegistry,
)
registry = StrategyRegistry()
registry.register(FakeStrategy("to-unregister"))
assert "to-unregister" in registry
registry.unregister("to-unregister")
assert "to-unregister" not in registry
def test_unregister_unknown_raises(self):
from cleveragents.application.services.strategy_registry import (
StrategyNotFoundError,
StrategyRegistry,
)
registry = StrategyRegistry()
with pytest.raises(StrategyNotFoundError):
registry.unregister("ghost")
def test_clear(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
for i in range(3):
registry.register(FakeStrategy(f"clear-{i}"))
assert len(registry) == 3
registry.clear()
assert len(registry) == 0
assert registry.list_all() == []
# ---------------------------------------------------------------------------
# Tests: Protocol conformance validation
# ---------------------------------------------------------------------------
class TestProtocolValidation:
"""Validate :meth:`StrategyRegistry.validate_registry` behavior."""
def test_validate_returns_no_warnings_empty(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
warnings = registry.validate_registry()
assert warnings == []
def test_validate_enabled_but_not_registered_warns(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
registry.inject_stale_enabled_entry("ghost-strategy")
warnings = registry.validate_registry()
assert any("ghost-strategy" in w for w in warnings)
def test_validate_domain_strategy_no_resource_types_warns(self):
"""Domain-model strategies must declare resource_types."""
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
strat = FakeStrategyWithNoCaps() # has domain caps with empty resource_types
registry.register(strat)
warnings = registry.validate_registry()
assert any("resource types" in w for w in warnings)
def test_validate_domain_strategy_with_resource_types_ok(self):
"""Domain strategies that declare resource_types pass validation."""
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
strat = FakeDomainStrategy("with-caps") # has resource_types=("text",)
registry.register(strat)
warnings = registry.validate_registry()
assert not any("resource types" in w for w in warnings)
def test_validate_v1_pipeline_strategies_skip_resource_types(self):
"""V1 pipeline strategies (with acms_service StrategyCapabilities) should NOT
get resource-types warnings because their capabilities object has no
such field. This was the fix introduced by #10590."""
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
# SimpleKeywordStrategy uses v1 pipeline StrategyCapabilities (acms_service)
# which lacks the domain-model resource_types field.
from cleveragents.application.services.context_strategies import (
SimpleKeywordStrategy,
)
registry.register(SimpleKeywordStrategy(), is_builtin=True)
warnings = registry.validate_registry()
# Should NOT have a "resource types" warning for v1 pipeline strategies
resource_warnings = [w for w in warnings if "resource types" in w]
assert len(resource_warnings) == 0
# ---------------------------------------------------------------------------
# Tests: Plugin loading from module:ClassName strings
# ---------------------------------------------------------------------------
class TestPluginLoading:
"""Test :meth:`StrategyRegistry.register_from_module`."""
def test_register_from_module_success(self):
from cleveragents.application.services.context_strategies import (
StrategyConfig,
StrategyRegistry,
)
registry = StrategyRegistry()
registry.register_from_module(
name="simple-keyword",
module_path="cleveragents.application.services.context_strategies:SimpleKeywordStrategy",
config=StrategyConfig(enabled=True),
)
assert "simple-keyword" in registry
strategy = registry.get("simple-keyword")
assert strategy.name == "simple-keyword"
def test_register_from_module_invalid_format_raises(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistrationError,
StrategyRegistry,
)
registry = StrategyRegistry()
with pytest.raises(StrategyRegistrationError, match="module:ClassName"):
registry.register_from_module(name="bad", module_path="invalid_format") # type: ignore[arg-type]
def test_register_from_module_unauthorized_prefix_raises(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistrationError,
StrategyRegistry,
)
registry = StrategyRegistry()
with pytest.raises(StrategyRegistrationError):
registry.register_from_module(
name="evil",
module_path="os.path:join", # type: ignore[arg-type]
)
def test_register_from_module_nonexistent_class_raises(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistrationError,
StrategyRegistry,
)
registry = StrategyRegistry()
with pytest.raises(StrategyRegistrationError, match="not found"):
registry.register_from_module(
name="bogus",
module_path="cleveragents.application.services.context_strategies:"
"NoSuchClassDoesNotExist", # type: ignore[arg-type]
)
# ---------------------------------------------------------------------------
# Tests: TOML configuration loading
# ---------------------------------------------------------------------------
class TestTOMLConfigurationLoading:
"""Test :func:`load_strategies_from_config`.
This exercises the entire pipeline: register built-ins, optionally load
custom plugins from module:ClassName strings, and set the enabled list.
"""
def test_load_empty_config_registers_defaults(self):
from cleveragents.application.services.context_strategies import (
load_strategies_from_config,
)
registry = load_strategies_from_config({}) # {} = minimal config
names = registry.list_all()
assert "simple-keyword" in names
assert "semantic-embedding" in names
assert "breadth-depth-navigator" in names
# Only defaults should be enabled
enabled = registry.list_enabled()
assert set(enabled) == {
"simple-keyword",
"semantic-embedding",
"breadth-depth-navigator",
}
def test_load_config_with_custom_plugin(self):
from cleveragents.application.services.context_strategies import (
load_strategies_from_config,
)
registry = load_strategies_from_config(
{
"custom": {
"my-strategy": "cleveragents.application.services.context_strategies:"
"SemanticEmbeddingStrategy",
},
}
)
assert "my-strategy" in registry
assert "simple-keyword" in registry
def test_load_config_with_custom_and_custom_enabled(self):
from cleveragents.application.services.context_strategies import (
load_strategies_from_config,
)
registry = load_strategies_from_config(
{
"enabled": ["simple-keyword", "my-strategy"],
"custom": {
"my-strategy": "cleveragents.application.services.context_strategies:"
"SemanticEmbeddingStrategy", # type: ignore[dict-item]
},
}
)
enabled = registry.list_enabled()
assert set(enabled) == {"simple-keyword", "my-strategy"}
def test_load_config_all_builtins_registered_as_builtin(self):
from cleveragents.application.services.context_strategies import (
load_strategies_from_config,
)
registry = load_strategies_from_config({})
for name in ["simple-keyword", "semantic-embedding", "breadth-depth-navigator"]:
entry = registry.get_entry(name)
assert entry.is_builtin is True
# ---------------------------------------------------------------------------
# Tests: Thread safety of registry operations
# ---------------------------------------------------------------------------
class TestThreadSafety:
"""Verify that concurrent register/get/set_enabled calls don't corrupt state."""
def test_concurrent_register(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
errors: list[Exception] = [] # type: ignore[var-annotated]
def register_worker(worker_id: int) -> None:
try:
for _ in range(50):
name = f"concurrent-strat-{worker_id}"
registry.register(FakeStrategy(name))
except Exception as exc: # noqa: BLE001
errors.append(exc)
threads = [threading.Thread(target=register_worker, args=(i,)) for i in range(10)]
for t in threads:
t.start()
for t in threads:
t.join()
assert not errors, f"Errors during concurrent register: {errors}"
# Each worker registers once (deduplicated within its own run)
total = len(registry)
assert total > 0
def test_concurrent_get_and_register(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
registry.register(FakeStrategy("shared")) # Pre-register one strategy
errors: list[Exception] = [] # type: ignore[var-annotated]
reads: list[str] = [] # type: ignore[var-annotated]
def reader(count: int) -> None:
for _ in range(20):
try:
s = registry.get("shared")
assert s is not None
reads.append(s.name)
except Exception as exc: # noqa: BLE001
errors.append(exc)
def writer(worker_id: int) -> None:
for _ in range(20):
name = f"writer-{worker_id}"
registry.register(FakeStrategy(name))
threads: list[threading.Thread] = []
threads.append(threading.Thread(target=reader, args=(1,)))
for i in range(5):
threads.append(threading.Thread(target=writer, args=(i,)))
for t in threads:
t.start()
for t in threads:
t.join()
assert not errors
assert len(reads) == 20
# ---------------------------------------------------------------------------
# Tests: Fallback degradation chain verification
# ---------------------------------------------------------------------------
class TestFallbackDegradation:
"""Verify that SimpleKeywordStrategy is always available as a fallback."""
def test_simple_keyword_always_produces_results(self):
from cleveragents.application.services.context_strategies import (
SimpleKeywordStrategy,
)
from cleveragents.domain.models.core.context_fragment import ContextBudget
strat = SimpleKeywordStrategy()
budget = ContextBudget(max_tokens=1000)
# Even with empty fragments, should return an empty list (no crash)
result = strat.assemble([], budget)
assert result == []
# Create a fake fragment-like object
class FakeFrag: # pylint: disable=C0115
relevance_score: float = 0.5
token_count: int = 10
uko_node: str = "project://test/node"
content: str = "hello world test fragment"
fragments = [FakeFrag()] # type: ignore[list-item]
result = strat.assemble(fragments, budget)
assert len(result) == 1
def test_simple_keyword_confidence_is_universal(self):
from cleveragents.application.services.context_strategies import (
SimpleKeywordStrategy,
)
strat = SimpleKeywordStrategy()
# Without query in request
assert strat.can_handle({}) == 0.3
# Request is always handled with at least 0.3 confidence
assert strat.can_handle({"query": ""}) == 0.3
assert strat.can_handle({"anything": "irrelevant"}) == 0.3
# ---------------------------------------------------------------------------
# Tests: Config-driven strategy enable/disable
# ---------------------------------------------------------------------------
class TestEnableDisable:
"""Test :meth:`StrategyRegistry.set_enabled` and config updates."""
def test_set_enabled_updates_list(self):
from cleveragents.application.services.strategy_registry import (
StrategyConfig,
StrategyRegistry,
)
registry = StrategyRegistry()
for name in ["a", "b", "c"]:
registry.register(FakeStrategy(name), config=StrategyConfig(enabled=True))
# Only enable 'a' and 'c'
registry.set_enabled(["a", "c"])
enabled = registry.list_enabled()
assert enabled == ["a", "c"]
def test_set_unknown_strategy_raises(self):
from cleveragents.application.services.strategy_registry import (
StrategyNotFoundError,
StrategyRegistry,
)
registry = StrategyRegistry()
registry.register(FakeStrategy("exists"))
with pytest.raises(StrategyNotFoundError, match="unknown"):
registry.set_enabled(["nonexistent"])
def test_set_enabled_updates_config_flags(self):
from cleveragents.application.services.strategy_registry import (
StrategyConfig,
StrategyRegistry,
)
registry = StrategyRegistry()
for name in ["x", "y"]:
registry.register(FakeStrategy(name))
# Initially enabled by default (enable_by_default=True)
assert registry.get_config("x").enabled is True
registry.set_enabled(["x"]) # disable y only
# Check config entries: x stays enabled, y should be disabled
assert registry.get_config("x").enabled is True
assert registry.get_config("y").enabled is False
def test_update_config_individual_fields(self):
from cleveragents.application.services.strategy_registry import (
StrategyConfig,
StrategyRegistry,
)
registry = StrategyRegistry()
registry.register(FakeStrategy("updatable"))
registry.update_config(
"updatable",
enabled=False,
timeout_seconds=120,
max_fragments=50,
)
cfg = registry.get_config("updatable")
assert cfg.enabled is False
assert cfg.timeout_seconds == 120
assert cfg.max_fragments == 50
def test_update_config_enable_re_adds_to_enabled_order(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
registry.register(FakeStrategy("re-enable"))
registry.update_config("re-enable", enabled=True)
assert "re-enable" in registry._enabled_order # pylint: disable=protected-access
def test_update_config_disable_removes_from_enabled_order(self):
from cleveragents.application.services.strategy_registry import (
StrategyRegistry,
)
registry = StrategyRegistry()
registry.register(FakeStrategy("disable-me"))
assert "disable-me" in registry._enabled_order # pylint: disable=protected-access
registry.update_config("disable-me", enabled=False)
assert "disable-me" not in registry._enabled_order # pylint: disable=protected-access
# ---------------------------------------------------------------------------
# Tests: Integration with acms_service auto-loading
# ---------------------------------------------------------------------------
class TestACMSPipelineAutoLoading:
"""Verify that ACMSPipeline.__init__ loads strategies from Settings."""
def test_pipeline_auto_loads_strategies_from_settings(self):
from unittest.mock import MagicMock, PropertyMock
# Create a mock Settings object with context.strategies config.
settings = MagicMock()
settings.context = { # type: ignore[attr-defined]
"strategies": {
"enabled": ["simple-keyword", "semantic-embedding"],
}
}
from cleveragents.application.services.acms_service import ACMSPipeline
pipeline = ACMSPipeline(settings=settings)
strategies_in_pipeline = list(pipeline._strategies.keys()) # pylint: disable=protected-access
# Built-in relevance strategy is always registered
assert "relevance" in strategies_in_pipeline
# Simple-keyword should be auto-loaded
assert "simple-keyword" in strategies_in_pipeline
def test_pipeline_works_without_settings(self):
from cleveragents.application.services.acms_service import ACMSPipeline
pipeline = ACMSPipeline()
assert "relevance" in pipeline._strategies # pylint: disable=protected-access