feat(skills): add skill package schema and agent-side skill loading support #94

Closed

CoreRasurae wants to merge 0 commits from feature/skill-package-support into master

pull from: feature/skill-package-support

merge into: cleveragents:master

cleveragents:master

cleveragents:tdd/m1-tool-max-rounds-precedence

cleveragents:feature/m1-rename-reasoning-config-property

cleveragents:feature/m1-inherit-tool-timeout-ceilings

cleveragents:bugfix/m1-executor-validate-configuration

cleveragents:bugfix/m1-registry-content-fetch

cleveragents:tdd/m1-registry-content-fetch

cleveragents:tdd/m1-registry-resolve-full-type-names

cleveragents:bugfix/m1-inline-sandbox-import-restriction

cleveragents:feature/m2-reasoning-model-provider-support

cleveragents:feature/76-budget-pre-flight

cleveragents:feature/17-public-api

cleveragents:feature/m1-registry-http-client

cleveragents:master-backup

Conversation 14 Commits - Files changed -

CoreRasurae commented 2026-07-31 21:24:58 +00:00

Member

Copy link

Summary

Implements #88: a normative Skill package schema (Package Registry Standard §16) aligned with the open agentskills.io Agent Skills format, plus agent-side skill loading for type: llm agents via a new optional skills config field, per the approved ADR-2034.

• SkillLoader, SkillValidator, SkillReferenceResolver (new): resolve skills references through the existing cleveractors.registry client (registry:/ID:/local: schemes), mirroring the established template package-reference resolution pattern rather than a parallel path.
• Progressive disclosure in three stages: discovery (system prompt), activation (skill tool call returns instructions), execution (resource reads, reusing file_read's offset/max_chars pagination from ADR-2033).
• AgentFactory invokes SkillLoader before LLMAgent construction; LLMAgent/ToolAgent gain no registry awareness — all resolution stays in SkillLoader.
• docs/actor-registry-standard.md gains a new §16 (appended after the original body, version bumped 1.0.0 → 1.1.0) documenting the schema. The skills agent-config field itself is documented only in ADR-2034 — not mirrored into docs/index.md — matching how ADR-2031/ADR-2032 handled their own new LLM agent config fields.
• docs/registry/ updated with pointers to the new schema.

Test plan

• 68 Behave scenarios (features/agent_skills.feature): schema validation (including all agentskills.io name/description constraints), all three reference schemes, loader orchestration, factory/agent integration, ToolAgent skill tool (activation, resource reads incl. base64, pagination, error paths).
• Robot Framework suite (robot/agent_skills.robot, 6 cases): real LocalPackageStore/PackageContentResolver/SkillLoader resolution against a real on-disk skill file, end to end through a real ToolAgent dispatch — only the LLM API call is mocked.
• nox -s coverage_report: 96.9% (threshold 96.5%).
• Full nox suite green (lint, format, typecheck, security_scan, dead_code, complexity, unit_tests, integration_tests, e2e_tests, coverage_report, docs, build).
• nox -s benchmark_regression: BENCHMARKS NOT SIGNIFICANTLY CHANGED.

Closes #88

## Summary Implements #88: a normative Skill package schema (Package Registry Standard §16) aligned with the open [agentskills.io](https://agentskills.io/specification) Agent Skills format, plus agent-side skill loading for `type: llm` agents via a new optional `skills` config field, per the approved ADR-2034. - `SkillLoader`, `SkillValidator`, `SkillReferenceResolver` (new): resolve `skills` references through the existing `cleveractors.registry` client (`registry:`/`ID:`/`local:` schemes), mirroring the established template package-reference resolution pattern rather than a parallel path. - Progressive disclosure in three stages: discovery (system prompt), activation (`skill` tool call returns instructions), execution (resource reads, reusing `file_read`'s `offset`/`max_chars` pagination from ADR-2033). - `AgentFactory` invokes `SkillLoader` before `LLMAgent` construction; `LLMAgent`/`ToolAgent` gain no registry awareness — all resolution stays in `SkillLoader`. - `docs/actor-registry-standard.md` gains a new §16 (appended after the original body, version bumped 1.0.0 → 1.1.0) documenting the schema. The `skills` agent-config field itself is documented only in ADR-2034 — not mirrored into `docs/index.md` — matching how ADR-2031/ADR-2032 handled their own new LLM agent config fields. - `docs/registry/` updated with pointers to the new schema. ## Test plan - [x] 68 Behave scenarios (`features/agent_skills.feature`): schema validation (including all agentskills.io name/description constraints), all three reference schemes, loader orchestration, factory/agent integration, ToolAgent skill tool (activation, resource reads incl. base64, pagination, error paths). - [x] Robot Framework suite (`robot/agent_skills.robot`, 6 cases): real `LocalPackageStore`/`PackageContentResolver`/`SkillLoader` resolution against a real on-disk skill file, end to end through a real `ToolAgent` dispatch — only the LLM API call is mocked. - [x] `nox -s coverage_report`: 96.9% (threshold 96.5%). - [x] Full `nox` suite green (lint, format, typecheck, security_scan, dead_code, complexity, unit_tests, integration_tests, e2e_tests, coverage_report, docs, build). - [x] `nox -s benchmark_regression`: BENCHMARKS NOT SIGNIFICANTLY CHANGED. Closes #88

CoreRasurae added this to the v2.1.0 milestone 2026-07-31 21:24:59 +00:00

CoreRasurae added a new dependency 2026-07-31 21:26:04 +00:00

#88 feat(skills): add Skill package schema and agent-side skill loading support

CoreRasurae added the

Type

Feature

label 2026-07-31 21:26:24 +00:00

CoreRasurae commented 2026-07-31 21:33:34 +00:00

Author

Member

Copy link

Note: bundled scripts/ resources are readable, not directly runnable

A question came up after this PR landed: if a skill's instructions say something like "run scripts/extract.py <file.pdf>", how does the type: llm agent actually execute that script? Documenting the answer here since it's not obvious from the code alone.

What the skill tool actually gives the model is text, not a runnable file. Calling skill(skill_name="pdf-processing", resource="scripts/extract.py") returns the script's source as a string (cleveractors.agents.tool.ToolAgent._skill_tool, commit bf1f138) — formatted as [SKILL_RESOURCE_READ]...[FILE_CONTENT_START]...[FILE_CONTENT_END]. Nothing is ever written to a real filesystem path; resources live only in the in-memory _loaded_skills mapping threaded through context["_skills"]. This is intentional, per ADR-2034 D-5: resources are exposed for execution-stage reads "without touching the host filesystem," not for by-path execution.

How a bundled Python script can actually run today — no code changes needed, but it requires the model to inline the source rather than reference it by path:

1. skill(skill_name="pdf-processing", resource="scripts/extract.py") → returns the source text
2. python_exec(code="<that source text>") → executes it via cleveractors.agents.tool.ToolAgent._execute_python_code

This only works when the agent config sets exec_python: true. Worth flagging: that sandbox isn't airtight — its restricted-builtins dict still includes an unrestricted __import__, so executed code can import os / import subprocess and go beyond the tool's documented builtin list. Pre-existing behavior, not something this PR changed, but relevant to anyone relying on it as a security boundary for skill-provided code.

What does not work today: a bundled .sh script, or any Python script that assumes it's a real file on disk (relative imports, sibling files, real argv, real cwd). There's no materialized file for shell or a real interpreter invocation to point at. A model could try to smuggle the whole script into bash -c '<script>' via the shell tool (needs allow_shell: true), but that's fragile and wasn't a designed path — it just happens to be technically possible given shell's existing arbitrary-command argument.

If genuine file-backed execution is wanted (skill resources materialized to a real temp directory so shell/python_exec/file_read can operate on real paths), that's a new capability this issue didn't build. It would need its own small design decision — materialization lifecycle, cleanup, interaction with safe_mode/unsafe_mode — similar in shape to the pack_skill_directory follow-up already flagged in ADR-2034's "Follow-up Required" section. Happy to scope that as a separate issue if it's wanted.

## Note: bundled `scripts/` resources are readable, not directly runnable A question came up after this PR landed: if a skill's `instructions` say something like *"run `scripts/extract.py <file.pdf>`"*, how does the `type: llm` agent actually execute that script? Documenting the answer here since it's not obvious from the code alone. **What the `skill` tool actually gives the model is text, not a runnable file.** Calling `skill(skill_name="pdf-processing", resource="scripts/extract.py")` returns the script's source as a string (`cleveractors.agents.tool.ToolAgent._skill_tool`, commit `bf1f138`) — formatted as `[SKILL_RESOURCE_READ]...[FILE_CONTENT_START]...[FILE_CONTENT_END]`. Nothing is ever written to a real filesystem path; resources live only in the in-memory `_loaded_skills` mapping threaded through `context["_skills"]`. This is intentional, per ADR-2034 D-5: resources are exposed for execution-stage reads "without touching the host filesystem," not for by-path execution. **How a bundled Python script can actually run today** — no code changes needed, but it requires the model to inline the source rather than reference it by path: 1. `skill(skill_name="pdf-processing", resource="scripts/extract.py")` → returns the source text 2. `python_exec(code="<that source text>")` → executes it via `cleveractors.agents.tool.ToolAgent._execute_python_code` This only works when the agent config sets `exec_python: true`. Worth flagging: that sandbox isn't airtight — its restricted-builtins dict still includes an unrestricted `__import__`, so executed code can `import os` / `import subprocess` and go beyond the tool's documented builtin list. Pre-existing behavior, not something this PR changed, but relevant to anyone relying on it as a security boundary for skill-provided code. **What does *not* work today:** a bundled `.sh` script, or any Python script that assumes it's a real file on disk (relative imports, sibling files, real argv, real cwd). There's no materialized file for `shell` or a real interpreter invocation to point at. A model could try to smuggle the whole script into `bash -c '<script>'` via the `shell` tool (needs `allow_shell: true`), but that's fragile and wasn't a designed path — it just happens to be technically possible given `shell`'s existing arbitrary-`command` argument. **If genuine file-backed execution is wanted** (skill resources materialized to a real temp directory so `shell`/`python_exec`/`file_read` can operate on real paths), that's a new capability this issue didn't build. It would need its own small design decision — materialization lifecycle, cleanup, interaction with `safe_mode`/`unsafe_mode` — similar in shape to the `pack_skill_directory` follow-up already flagged in ADR-2034's "Follow-up Required" section. Happy to scope that as a separate issue if it's wanted.

hurui200320 requested changes 2026-08-01 09:59:16 +00:00

Dismissed

hurui200320 left a comment

Copy link

PR Review: !94 (Ticket #88)

Verdict: Request Changes

The implementation is solid overall and closely follows ADR-2034, reusing the existing registry client and preserving progressive disclosure. However, two functional issues require fixes before approval: skill tool context is lost in the stuck-model synthesis path, and duplicate skill names are silently overwritten. Additionally, there are a few minor documentation/coverage observations.

Critical Issues

None

Major Issues

1. src/cleveractors/agents/llm.py lines 1429-1431 — The stuck-model synthesis follow-up constructs tool_ctx_s as {"_unsafe_mode": True} if parent_unsafe else None instead of calling self._build_tool_context(parent_unsafe). Consequently, _loaded_skills is not forwarded to the ephemeral ToolAgent. If the model emits a skill tool call during this follow-up round, ToolAgent._skill_tool fails with "Skill '...' is not loaded for this agent." The main loop and the budget-exhaustion synthesis round both correctly use _build_tool_context; this path should too.

2. src/cleveractors/agents/skills.py lines 98-109 — SkillLoader.load stores resolved skills in loaded[skill.name] = skill, silently overwriting any earlier skill with the same name. Two skills references that resolve to identically-named skills (or two refs to the same skill) will yield a catalogue with one entry and the other's instructions will be lost. Validate uniqueness and raise AgentCreationError with a clear message.

Minor Issues

1. Coverage threshold discrepancy — The PR test plan reports 96.9% coverage and cites a 96.5% threshold, but CONTRIBUTING.md, .gitea/workflows/ci.yml, and the coverage_report session docstring all state the gate is 97%. The noxfile.py constant is 96.5, which creates an inconsistency. Before merge, confirm the project's enforced gate and ensure the reported coverage meets it. If the gate is 97%, additional tests are needed.

2. docs/actor-registry-standard.md §16.1 — The last paragraph references "Actor Configuration Standard §22" for the agent-side skills field, but docs/index.md has no §22. This appears to be a stale/future reference; it should point to the actual location (currently ADR-2034, or §4.4 once the deferred spec update lands).

3. src/cleveractors/agents/skill_schema.py Skill.to_context_dict — The context representation exposed to the skill tool and LLMAgent.get_metadata() drops metadata, allowed_tools, license, and compatibility. Since allowed_tools is documented as a pre-approved tool list, making it invisible to the model limits its usefulness. Consider surfacing at least allowed_tools and metadata in activation output or catalogue metadata.

Nits

1. Test gaps — Consider adding Behave/Robot scenarios for: duplicate skill names in the skills list; empty description/instructions strings; and the stuck-model synthesis path invoking the skill tool.
2. robot/SkillLoadingTestLib.py — tempfile.mkdtemp() directories are not cleaned up after tests.

Summary

The PR delivers the normative skill schema, reference resolution, loader orchestration, and tool-agent integration described in ADR-2034. The architecture is clean and the test suite is extensive. The two major issues above are localized but real regressions in functionality; once fixed and the coverage/doc observations addressed, this should be good to merge.

## PR Review: !94 (Ticket #88) ### Verdict: Request Changes The implementation is solid overall and closely follows ADR-2034, reusing the existing registry client and preserving progressive disclosure. However, two functional issues require fixes before approval: skill tool context is lost in the stuck-model synthesis path, and duplicate skill names are silently overwritten. Additionally, there are a few minor documentation/coverage observations. ### Critical Issues None ### Major Issues 1. **`src/cleveractors/agents/llm.py` lines 1429-1431** — The stuck-model synthesis follow-up constructs `tool_ctx_s` as `{"_unsafe_mode": True} if parent_unsafe else None` instead of calling `self._build_tool_context(parent_unsafe)`. Consequently, `_loaded_skills` is not forwarded to the ephemeral `ToolAgent`. If the model emits a `skill` tool call during this follow-up round, `ToolAgent._skill_tool` fails with "Skill '...' is not loaded for this agent." The main loop and the budget-exhaustion synthesis round both correctly use `_build_tool_context`; this path should too. 2. **`src/cleveractors/agents/skills.py` lines 98-109** — `SkillLoader.load` stores resolved skills in `loaded[skill.name] = skill`, silently overwriting any earlier skill with the same name. Two `skills` references that resolve to identically-named skills (or two refs to the same skill) will yield a catalogue with one entry and the other's instructions will be lost. Validate uniqueness and raise `AgentCreationError` with a clear message. ### Minor Issues 1. **Coverage threshold discrepancy** — The PR test plan reports 96.9% coverage and cites a 96.5% threshold, but `CONTRIBUTING.md`, `.gitea/workflows/ci.yml`, and the `coverage_report` session docstring all state the gate is 97%. The `noxfile.py` constant is 96.5, which creates an inconsistency. Before merge, confirm the project's enforced gate and ensure the reported coverage meets it. If the gate is 97%, additional tests are needed. 2. **`docs/actor-registry-standard.md` §16.1** — The last paragraph references "Actor Configuration Standard §22" for the agent-side `skills` field, but `docs/index.md` has no §22. This appears to be a stale/future reference; it should point to the actual location (currently ADR-2034, or §4.4 once the deferred spec update lands). 3. **`src/cleveractors/agents/skill_schema.py` `Skill.to_context_dict`** — The context representation exposed to the `skill` tool and `LLMAgent.get_metadata()` drops `metadata`, `allowed_tools`, `license`, and `compatibility`. Since `allowed_tools` is documented as a pre-approved tool list, making it invisible to the model limits its usefulness. Consider surfacing at least `allowed_tools` and `metadata` in activation output or catalogue metadata. ### Nits 1. **Test gaps** — Consider adding Behave/Robot scenarios for: duplicate skill names in the `skills` list; empty `description`/`instructions` strings; and the stuck-model synthesis path invoking the `skill` tool. 2. **`robot/SkillLoadingTestLib.py`** — `tempfile.mkdtemp()` directories are not cleaned up after tests. ### Summary The PR delivers the normative skill schema, reference resolution, loader orchestration, and tool-agent integration described in ADR-2034. The architecture is clean and the test suite is extensive. The two major issues above are localized but real regressions in functionality; once fixed and the coverage/doc observations addressed, this should be good to merge.

CoreRasurae force-pushed feature/skill-package-support from bf1f13846f to 05e5e52310 2026-08-01 15:05:03 +00:00 Compare

CoreRasurae force-pushed feature/skill-package-support from c520a6d211 to f9fea9e208 2026-08-01 15:41:02 +00:00 Compare

hurui200320 requested changes 2026-08-02 13:29:46 +00:00

Dismissed

hurui200320 left a comment

Copy link

PR Review: !94 (Ticket #88)

Verdict: Request Changes

The implementation still has the two functional regressions identified in the previous review: the stuck-model synthesis path drops the skill catalogue, and duplicate skill names are silently overwritten. In addition, the agent-side skills field is not documented in docs/index.md as required by the ticket acceptance criteria, and a new error-handling bug in SkillReferenceResolver can mask exceptions with UnboundLocalError. Once these are addressed, the PR should be in good shape.

Critical Issues

None

Major Issues

1. src/cleveractors/agents/llm.py lines 1429-1431 — The stuck-model synthesis follow-up constructs tool_ctx_s as {"_unsafe_mode": True} if parent_unsafe else None instead of calling self._build_tool_context(parent_unsafe). Consequently, _loaded_skills is not forwarded to the ephemeral ToolAgent. If the model emits a skill tool call during this follow-up round, ToolAgent._skill_tool fails with "Skill '...' is not loaded for this agent." The main loop and budget-exhaustion synthesis round both correctly use _build_tool_context; this path should too.

2. src/cleveractors/agents/skills.py lines 98-109 — SkillLoader.load stores resolved skills in loaded[skill.name] = skill, silently overwriting any earlier skill with the same name. Two skills references that resolve to identically-named skills (or two refs to the same skill) will yield a catalogue with one entry and the other's instructions will be lost. Validate uniqueness and raise AgentCreationError with a clear message.

3. docs/index.md §4.4 — The new optional skills configuration field for type: llm agents is not documented in the Actor Configuration Standard, even though ticket #88 acceptance criteria explicitly require it and ADR-2034 D-5 states the field is added to docs/index.md §4.4. Add the field documentation (optional list of package references using registry:, ID:, and local: schemes).

4. src/cleveractors/agents/skill_resolution.py lines 84-124 — resolved is only assigned inside the try block. If resolver.resolve() raises an exception other than FileNotFoundError/ValueError (e.g., RuntimeError for a registry reference called inside a running event loop), the finally runs and then if resolved is None: raises UnboundLocalError, masking the original failure. Initialize resolved = None before the try or add a broader except path that re-raises as RegistryError.

Minor Issues

5. Coverage threshold mismatch / below declared gate — CONTRIBUTING.md, .gitea/workflows/ci.yml, and the coverage_report session docstring state the gate is 97%, but noxfile.py sets COVERAGE_THRESHOLD = 96.5. The PR test plan reports 96.9% and cites 96.5%. Align noxfile.py with the documented 97% gate and add tests to reach it.

6. docs/actor-registry-standard.md §16.1 line 576 — The last paragraph references "Actor Configuration Standard §22" for the agent-side skills field, but docs/index.md has no §22. This appears to be a stale/future reference; it should point to the actual location (currently §4.4 or ADR-2034).

7. src/cleveractors/agents/skill_schema.py Skill.to_context_dict lines 72-88 — The context representation exposed to the skill tool and LLMAgent.get_metadata() drops metadata, allowed_tools, license, and compatibility. Since allowed_tools is documented as a pre-approved tool list, making it invisible to the model limits its usefulness. Consider surfacing at least allowed_tools and metadata in activation output or catalogue metadata.

8. src/cleveractors/agents/tool.py _decode_skill_resource line 1099 and src/cleveractors/agents/skill_schema.py SkillResource.decoded_text line 53 — Base64 resources are decoded with .decode("utf-8"), which will crash for genuine binary assets (images, PDFs, etc.) even though the spec requires supporting non-UTF-8 files via encoding: base64. Handle binary resources safely (e.g., return base64 text or skip UTF-8 decoding).

9. robot/SkillLoadingTestLib.py line 56 — tempfile.mkdtemp() directories are not cleaned up after tests. Add teardown cleanup to avoid leaking temp directories in CI.

Nits

10. Test gaps — Consider adding Behave/Robot scenarios for: duplicate skill names in the skills list; empty description/instructions strings; and the stuck-model synthesis path invoking the skill tool.

11. features/steps/agent_skills_steps.py — tempfile.mkdtemp() directories created for local-store tests are not cleaned up.

12. src/cleveractors/agents/skill_resolution.py lines 94-112 — When called inside a running event loop, resolver cleanup is scheduled with asyncio.ensure_future() and not awaited, so it may not complete before loop shutdown. Prefer the async resolution path where possible.

Summary

The PR delivers a clean, registry-agnostic skill-loading architecture that aligns well with ADR-2034 and reuses the existing package-resolution machinery. The test suite is extensive and the Robot integration test gives good end-to-end confidence. However, the two previously reported functional issues remain unfixed, the new SkillReferenceResolver error-handling path has a latent UnboundLocalError, and the skills field is missing from the Actor Configuration Standard. Addressing the major items above will make this ready to merge.

## PR Review: !94 (Ticket #88) ### Verdict: Request Changes The implementation still has the two functional regressions identified in the previous review: the stuck-model synthesis path drops the skill catalogue, and duplicate skill names are silently overwritten. In addition, the agent-side `skills` field is not documented in `docs/index.md` as required by the ticket acceptance criteria, and a new error-handling bug in `SkillReferenceResolver` can mask exceptions with `UnboundLocalError`. Once these are addressed, the PR should be in good shape. ### Critical Issues None ### Major Issues 1. **`src/cleveractors/agents/llm.py` lines 1429-1431** — The stuck-model synthesis follow-up constructs `tool_ctx_s` as `{"_unsafe_mode": True} if parent_unsafe else None` instead of calling `self._build_tool_context(parent_unsafe)`. Consequently, `_loaded_skills` is not forwarded to the ephemeral `ToolAgent`. If the model emits a `skill` tool call during this follow-up round, `ToolAgent._skill_tool` fails with "Skill '...' is not loaded for this agent." The main loop and budget-exhaustion synthesis round both correctly use `_build_tool_context`; this path should too. 2. **`src/cleveractors/agents/skills.py` lines 98-109** — `SkillLoader.load` stores resolved skills in `loaded[skill.name] = skill`, silently overwriting any earlier skill with the same name. Two `skills` references that resolve to identically-named skills (or two refs to the same skill) will yield a catalogue with one entry and the other's instructions will be lost. Validate uniqueness and raise `AgentCreationError` with a clear message. 3. **`docs/index.md` §4.4** — The new optional `skills` configuration field for `type: llm` agents is not documented in the Actor Configuration Standard, even though ticket #88 acceptance criteria explicitly require it and ADR-2034 D-5 states the field is added to `docs/index.md` §4.4. Add the field documentation (optional list of package references using `registry:`, `ID:`, and `local:` schemes). 4. **`src/cleveractors/agents/skill_resolution.py` lines 84-124** — `resolved` is only assigned inside the `try` block. If `resolver.resolve()` raises an exception other than `FileNotFoundError`/`ValueError` (e.g., `RuntimeError` for a registry reference called inside a running event loop), the `finally` runs and then `if resolved is None:` raises `UnboundLocalError`, masking the original failure. Initialize `resolved = None` before the `try` or add a broader `except` path that re-raises as `RegistryError`. ### Minor Issues 5. **Coverage threshold mismatch / below declared gate** — `CONTRIBUTING.md`, `.gitea/workflows/ci.yml`, and the `coverage_report` session docstring state the gate is **97%**, but `noxfile.py` sets `COVERAGE_THRESHOLD = 96.5`. The PR test plan reports **96.9%** and cites 96.5%. Align `noxfile.py` with the documented 97% gate and add tests to reach it. 6. **`docs/actor-registry-standard.md` §16.1 line 576** — The last paragraph references "Actor Configuration Standard §22" for the agent-side `skills` field, but `docs/index.md` has no §22. This appears to be a stale/future reference; it should point to the actual location (currently §4.4 or ADR-2034). 7. **`src/cleveractors/agents/skill_schema.py` `Skill.to_context_dict` lines 72-88** — The context representation exposed to the `skill` tool and `LLMAgent.get_metadata()` drops `metadata`, `allowed_tools`, `license`, and `compatibility`. Since `allowed_tools` is documented as a pre-approved tool list, making it invisible to the model limits its usefulness. Consider surfacing at least `allowed_tools` and `metadata` in activation output or catalogue metadata. 8. **`src/cleveractors/agents/tool.py` `_decode_skill_resource` line 1099 and `src/cleveractors/agents/skill_schema.py` `SkillResource.decoded_text` line 53** — Base64 resources are decoded with `.decode("utf-8")`, which will crash for genuine binary assets (images, PDFs, etc.) even though the spec requires supporting non-UTF-8 files via `encoding: base64`. Handle binary resources safely (e.g., return base64 text or skip UTF-8 decoding). 9. **`robot/SkillLoadingTestLib.py` line 56** — `tempfile.mkdtemp()` directories are not cleaned up after tests. Add teardown cleanup to avoid leaking temp directories in CI. ### Nits 10. **Test gaps** — Consider adding Behave/Robot scenarios for: duplicate skill names in the `skills` list; empty `description`/`instructions` strings; and the stuck-model synthesis path invoking the `skill` tool. 11. **`features/steps/agent_skills_steps.py`** — `tempfile.mkdtemp()` directories created for local-store tests are not cleaned up. 12. **`src/cleveractors/agents/skill_resolution.py` lines 94-112** — When called inside a running event loop, resolver cleanup is scheduled with `asyncio.ensure_future()` and not awaited, so it may not complete before loop shutdown. Prefer the async resolution path where possible. ### Summary The PR delivers a clean, registry-agnostic skill-loading architecture that aligns well with ADR-2034 and reuses the existing package-resolution machinery. The test suite is extensive and the Robot integration test gives good end-to-end confidence. However, the two previously reported functional issues remain unfixed, the new `SkillReferenceResolver` error-handling path has a latent `UnboundLocalError`, and the `skills` field is missing from the Actor Configuration Standard. Addressing the major items above will make this ready to merge.

CoreRasurae commented 2026-08-03 09:22:32 +00:00

Author

Member

Copy link

In the review report issuecomment 321235, Minor Issues, Issue 5, the coverage threshold does not need to be changed the coverage threshold is set to 96.5% because nox coverage does no rounding. The 97% coverage specified as the target value is after rounding, so 96.5% real coverage will meet that target. There is no need to change anything,

In the review report issuecomment 321235, Minor Issues, Issue 5, the coverage threshold does not need to be changed the coverage threshold is set to 96.5% because nox coverage does no rounding. The 97% coverage specified as the target value is after rounding, so 96.5% real coverage will meet that target. There is no need to change anything,

CoreRasurae force-pushed feature/skill-package-support from f9fea9e208 to 4660e373f2 2026-08-03 10:31:58 +00:00 Compare

CoreRasurae commented 2026-08-03 10:35:08 +00:00

Author

Member

Copy link

Addressed the outstanding review items from the 2026-08-02 review (commit 4660e37):

Fixed:

1. llm.py stuck-model synthesis follow-up now calls self._build_tool_context(parent_unsafe) instead of hand-building the context dict, so _loaded_skills is forwarded correctly. Added a regression scenario (Stuck-model synthesis forwards the loaded skill catalogue to the skill tool, features/llm_agent_tool_loop.feature) that fails without the fix.
2. SkillLoader.load now raises AgentCreationError on duplicate skill names instead of silently overwriting.
3. skill_resolution.py — see "Not changed" below; this one turned out not to be a real bug.
4. docs/actor-registry-standard.md §16.1's stale "Actor Configuration Standard §22" reference now points to ADR-2034 (see "Not changed" #3 for why not §4.4).
5. Skill.to_context_dict() now surfaces license, compatibility, metadata, and allowed_tools.
6. Base64-encoded resources that aren't valid UTF-8 (genuine binary) now decode safely (fall back to the original base64 text) instead of raising UnicodeDecodeError, in both SkillResource.decoded_text() and ToolAgent._decode_skill_resource.
7. robot/SkillLoadingTestLib.py and features/steps/agent_skills_steps.py temp dirs are now cleaned up (Test Teardown / context.add_cleanup).

Added Behave coverage for all of the above; full nox suite (lint, format, typecheck, unit_tests, coverage, security_scan, dead_code, integration_tests) is green.

Not changed (reviewed against the ADR/issue and believe these aren't correct):

1. docs/index.md §4.4 documentation — checked the actual file: none of ADR-2031's or ADR-2032's new LLM-agent config fields (token_budget_percent, max_retries, max_retry_time, etc.) were ever mirrored into §4.4 either — they live in their ADRs only, graduation into the normative spec is a deliberate future step. The skills field follows the same established pattern; the PR description already calls this out explicitly.
2. skill_resolution.py UnboundLocalError — traced through the actual control flow: if resolver.resolve() raises anything not caught by except (FileNotFoundError, ValueError), that exception propagates immediately once finally completes — the if resolved is None: line is never reached, so resolved is never read unbound. Verified this with a standalone repro of the same try/except/finally shape. No UnboundLocalError is possible here.
3. Coverage threshold — as already noted in issuecomment 321411, no change needed; unrelated to this PR (the noxfile.py constant predates this branch).

Not fixed (intentionally, out of scope for this PR): the asyncio.ensure_future() fire-and-forget cleanup in skill_resolution.py (Nit #12) — this is an existing, documented trade-off for the sync/async bridging in that function; addressing it properly would need a broader async-path refactor beyond this issue's scope.

Addressed the outstanding review items from the 2026-08-02 review (commit 4660e37): **Fixed:** 1. `llm.py` stuck-model synthesis follow-up now calls `self._build_tool_context(parent_unsafe)` instead of hand-building the context dict, so `_loaded_skills` is forwarded correctly. Added a regression scenario (`Stuck-model synthesis forwards the loaded skill catalogue to the skill tool`, `features/llm_agent_tool_loop.feature`) that fails without the fix. 2. `SkillLoader.load` now raises `AgentCreationError` on duplicate skill names instead of silently overwriting. 3. `skill_resolution.py` — see "Not changed" below; this one turned out not to be a real bug. 4. `docs/actor-registry-standard.md` §16.1's stale "Actor Configuration Standard §22" reference now points to ADR-2034 (see "Not changed" #3 for why not §4.4). 5. `Skill.to_context_dict()` now surfaces `license`, `compatibility`, `metadata`, and `allowed_tools`. 6. Base64-encoded resources that aren't valid UTF-8 (genuine binary) now decode safely (fall back to the original base64 text) instead of raising `UnicodeDecodeError`, in both `SkillResource.decoded_text()` and `ToolAgent._decode_skill_resource`. 7. `robot/SkillLoadingTestLib.py` and `features/steps/agent_skills_steps.py` temp dirs are now cleaned up (`Test Teardown` / `context.add_cleanup`). Added Behave coverage for all of the above; full `nox` suite (lint, format, typecheck, unit_tests, coverage, security_scan, dead_code, integration_tests) is green. **Not changed (reviewed against the ADR/issue and believe these aren't correct):** 1. **`docs/index.md` §4.4 documentation** — checked the actual file: none of ADR-2031's or ADR-2032's new LLM-agent config fields (`token_budget_percent`, `max_retries`, `max_retry_time`, etc.) were ever mirrored into §4.4 either — they live in their ADRs only, graduation into the normative spec is a deliberate future step. The `skills` field follows the same established pattern; the PR description already calls this out explicitly. 2. **`skill_resolution.py` `UnboundLocalError`** — traced through the actual control flow: if `resolver.resolve()` raises anything not caught by `except (FileNotFoundError, ValueError)`, that exception propagates immediately once `finally` completes — the `if resolved is None:` line is never reached, so `resolved` is never read unbound. Verified this with a standalone repro of the same try/except/finally shape. No `UnboundLocalError` is possible here. 3. **Coverage threshold** — as already noted in issuecomment 321411, no change needed; unrelated to this PR (the `noxfile.py` constant predates this branch). **Not fixed (intentionally, out of scope for this PR):** the `asyncio.ensure_future()` fire-and-forget cleanup in `skill_resolution.py` (Nit #12) — this is an existing, documented trade-off for the sync/async bridging in that function; addressing it properly would need a broader async-path refactor beyond this issue's scope.

hurui200320 requested changes 2026-08-03 13:10:49 +00:00

Dismissed

hurui200320 left a comment

Copy link

PR Review: !94 (Ticket #88)

Verdict: Request Changes

The two functional regressions from the previous review (stuck-model synthesis dropping the skill catalogue and duplicate skill names being silently overwritten) are fixed in the latest commit. The remaining concern is a new functional gap: registry: skill references cannot actually be resolved in the production async runtime path, because SkillLoader/SkillReferenceResolver are synchronous and call PackageContentResolver.resolve() from inside a running event loop. There are also two smaller robustness/efficiency observations.

Critical Issues

None

Major Issues

1. src/cleveractors/agents/skill_resolution.py lines 52–124 / src/cleveractors/registry/reference_resolver.py lines 557–588 / src/cleveractors/runtime_dispatch.py lines 162–168, 390–391, 850–851, 1252–1253
   SkillReferenceResolver.resolve() is synchronous. When it reaches a registry: reference it delegates to PackageContentResolver.resolve(), whose _RegistryReferenceStrategy checks for a running event loop and raises RuntimeError because it cannot nest asyncio.run(). SkillLoader.load() is invoked synchronously from AgentFactory._create_agent_instance(), which is called from the async _execute_llm, _execute_graph, _execute_llm_stream, and _execute_graph_stream dispatch functions. Consequently, any type: llm agent configured with a registry: skill reference fails at agent-creation time with an unhandled RuntimeError instead of loading the skill. The existing Behave scenario for registry: mocks the resolver, and the Robot suite only exercises local:, so this gap is not covered.
   Recommendation: Add an async resolution path (e.g. SkillReferenceResolver.aresolve() and SkillLoader.aload()) and use it from the async runtime dispatch, or resolve skills earlier (e.g. during Executor construction) before the event loop is running.

Minor Issues

1. src/cleveractors/agents/skill_resolution.py lines 79–118 / src/cleveractors/runtime_dispatch.py lines 873–890
   SkillReferenceResolver.resolve() creates a fresh ad-hoc PackageContentResolver for every skill reference when no resolver is injected (the runtime path constructed by _build_skill_loader()). When called inside the running event loop, the cleanup close_all() is scheduled with asyncio.ensure_future() and not awaited. This duplicates work and leaks client pools per skill reference.
   Recommendation: Have _build_skill_loader() create one shared PackageContentResolver and pass it to SkillReferenceResolver; provide an async cleanup path that is awaited.

2. src/cleveractors/agents/skill_schema.py lines 266–319 / src/cleveractors/agents/tool.py lines 331–344
   _validate_resources() accepts encoding: base64 without validating that content is valid base64. A malformed skill package therefore passes load-time validation but crashes at execution time when base64.b64decode() raises binascii.Error. There is no test coverage for invalid base64 content.
   Recommendation: Validate base64 content inside _validate_resources() and raise ValidationError with a clear message.

Nits

None

Summary

The PR now correctly handles duplicate skill names and forwards the loaded skill catalogue through the stuck-model synthesis path, resolving the blockers from the prior review. The architecture (factory-level SkillLoader, registry-agnostic LLMAgent, progressive disclosure via the synthesized skill tool) remains clean and well tested for local: and mocked paths. However, the production async runtime path cannot resolve registry: skill references, which is a real functional gap for one of the three advertised reference schemes. Fixing that (and the related resolver-per-reference overhead and base64 validation gap) will make this ready to merge.

Note: per your follow-up comments, I have not re-raised the docs/index.md §4.4 update or the coverage-threshold discussion, as you have explicitly treated those as deferred/out-of-scope.

## PR Review: !94 (Ticket #88) ### Verdict: Request Changes The two functional regressions from the previous review (stuck-model synthesis dropping the skill catalogue and duplicate skill names being silently overwritten) are fixed in the latest commit. The remaining concern is a new functional gap: `registry:` skill references cannot actually be resolved in the production async runtime path, because `SkillLoader`/`SkillReferenceResolver` are synchronous and call `PackageContentResolver.resolve()` from inside a running event loop. There are also two smaller robustness/efficiency observations. ### Critical Issues None ### Major Issues 1. **`src/cleveractors/agents/skill_resolution.py` lines 52–124 / `src/cleveractors/registry/reference_resolver.py` lines 557–588 / `src/cleveractors/runtime_dispatch.py` lines 162–168, 390–391, 850–851, 1252–1253** `SkillReferenceResolver.resolve()` is synchronous. When it reaches a `registry:` reference it delegates to `PackageContentResolver.resolve()`, whose `_RegistryReferenceStrategy` checks for a running event loop and raises `RuntimeError` because it cannot nest `asyncio.run()`. `SkillLoader.load()` is invoked synchronously from `AgentFactory._create_agent_instance()`, which is called from the async `_execute_llm`, `_execute_graph`, `_execute_llm_stream`, and `_execute_graph_stream` dispatch functions. Consequently, any `type: llm` agent configured with a `registry:` skill reference fails at agent-creation time with an unhandled `RuntimeError` instead of loading the skill. The existing Behave scenario for `registry:` mocks the resolver, and the Robot suite only exercises `local:`, so this gap is not covered. **Recommendation:** Add an async resolution path (e.g. `SkillReferenceResolver.aresolve()` and `SkillLoader.aload()`) and use it from the async runtime dispatch, or resolve `skills` earlier (e.g. during `Executor` construction) before the event loop is running. ### Minor Issues 1. **`src/cleveractors/agents/skill_resolution.py` lines 79–118 / `src/cleveractors/runtime_dispatch.py` lines 873–890** `SkillReferenceResolver.resolve()` creates a fresh ad-hoc `PackageContentResolver` for every skill reference when no resolver is injected (the runtime path constructed by `_build_skill_loader()`). When called inside the running event loop, the cleanup `close_all()` is scheduled with `asyncio.ensure_future()` and not awaited. This duplicates work and leaks client pools per skill reference. **Recommendation:** Have `_build_skill_loader()` create one shared `PackageContentResolver` and pass it to `SkillReferenceResolver`; provide an async cleanup path that is awaited. 2. **`src/cleveractors/agents/skill_schema.py` lines 266–319 / `src/cleveractors/agents/tool.py` lines 331–344** `_validate_resources()` accepts `encoding: base64` without validating that `content` is valid base64. A malformed skill package therefore passes load-time validation but crashes at execution time when `base64.b64decode()` raises `binascii.Error`. There is no test coverage for invalid base64 content. **Recommendation:** Validate base64 content inside `_validate_resources()` and raise `ValidationError` with a clear message. ### Nits None ### Summary The PR now correctly handles duplicate skill names and forwards the loaded skill catalogue through the stuck-model synthesis path, resolving the blockers from the prior review. The architecture (factory-level `SkillLoader`, registry-agnostic `LLMAgent`, progressive disclosure via the synthesized `skill` tool) remains clean and well tested for `local:` and mocked paths. However, the production async runtime path cannot resolve `registry:` skill references, which is a real functional gap for one of the three advertised reference schemes. Fixing that (and the related resolver-per-reference overhead and base64 validation gap) will make this ready to merge. Note: per your follow-up comments, I have not re-raised the `docs/index.md` §4.4 update or the coverage-threshold discussion, as you have explicitly treated those as deferred/out-of-scope.

CoreRasurae force-pushed feature/skill-package-support from 4660e373f2 to 3e33656c3f 2026-08-03 14:13:10 +00:00 Compare

CoreRasurae commented 2026-08-03 14:13:26 +00:00

Author

Member

Copy link

Addressed the outstanding review items from the 2026-08-03 review (commit 3e33656):

Fixed:

1. Major — registry: skill references now resolve from the async runtime dispatch path. Added SkillReferenceResolver.aresolve() / SkillLoader.aload() (mirroring PackageContentResolver.aresolve(), per ADR-2034 D-5) and AgentFactory.acreate_agent() / _acreate_agent_instance(); _execute_llm/_execute_graph/_execute_llm_stream/_execute_graph_stream now call await factory.acreate_agent(...) instead of the sync create_agent(), so a registry: reference no longer hits RuntimeError: asyncio.run() cannot be called from a running event loop.
2. Minor — _build_skill_loader() now constructs one shared PackageContentResolver per dispatch call (instead of one ad hoc per skill reference) and it is closed via an awaited close_all() alongside each dispatch function's existing agent-cleanup finally block.
3. Minor — SkillValidator._validate_resources() now validates that encoding: base64 resource content actually decodes as well-formed base64 (binascii.Error -> ValidationError) instead of only surfacing the problem later at execution time.

Added 14 new Behave scenarios covering the async resolution path (including a regression scenario proving the old sync path raises RuntimeError from a running loop), the shared-resolver reuse, an end-to-end run through the real Executor/runtime-dispatch layer with a real on-disk skill package, and malformed base64 rejection. Full nox suite green (lint, format, typecheck, security_scan, dead_code, unit_tests — 2876 scenarios, coverage on the touched files 91-100%).

As before, the docs/index.md §4.4 documentation and coverage-threshold points remain out of scope per the prior discussion (issuecomment 321411, issuecomment 321423).

Addressed the outstanding review items from the 2026-08-03 review (commit 3e33656): **Fixed:** 1. **Major** — `registry:` skill references now resolve from the async runtime dispatch path. Added `SkillReferenceResolver.aresolve()` / `SkillLoader.aload()` (mirroring `PackageContentResolver.aresolve()`, per ADR-2034 D-5) and `AgentFactory.acreate_agent()` / `_acreate_agent_instance()`; `_execute_llm`/`_execute_graph`/`_execute_llm_stream`/`_execute_graph_stream` now call `await factory.acreate_agent(...)` instead of the sync `create_agent()`, so a `registry:` reference no longer hits `RuntimeError: asyncio.run() cannot be called from a running event loop`. 2. **Minor** — `_build_skill_loader()` now constructs one shared `PackageContentResolver` per dispatch call (instead of one ad hoc per skill reference) and it is closed via an awaited `close_all()` alongside each dispatch function's existing agent-cleanup `finally` block. 3. **Minor** — `SkillValidator._validate_resources()` now validates that `encoding: base64` resource content actually decodes as well-formed base64 (`binascii.Error` -> `ValidationError`) instead of only surfacing the problem later at execution time. Added 14 new Behave scenarios covering the async resolution path (including a regression scenario proving the old sync path raises `RuntimeError` from a running loop), the shared-resolver reuse, an end-to-end run through the real `Executor`/runtime-dispatch layer with a real on-disk skill package, and malformed base64 rejection. Full `nox` suite green (lint, format, typecheck, security_scan, dead_code, unit_tests — 2876 scenarios, coverage on the touched files 91-100%). As before, the `docs/index.md` §4.4 documentation and coverage-threshold points remain out of scope per the prior discussion (issuecomment 321411, issuecomment 321423).

hurui200320 requested changes 2026-08-03 14:53:54 +00:00

Dismissed

hurui200320 left a comment

Copy link

PR Review: !94 (Ticket #88)

Verdict: Request Changes

The implementation is functionally complete and addresses the blockers from the prior review rounds: the async resolution path now makes registry: skill references work from the production runtime dispatch, duplicate skill names are rejected, the loaded skill catalogue is forwarded through the stuck-model synthesis follow-up, and base64/binary resources are handled safely. The test suite is extensive and the Robot integration test gives good end-to-end confidence.

I did not re-raise the docs/index.md §4.4 update or the coverage-threshold discussion, since you have explicitly treated those as deferred/out-of-scope in your follow-up comments.

There is one remaining error-path cleanup bug in the single-LLM dispatch path that should be fixed before merge.

Critical Issues

None

Major Issues

1. src/cleveractors/runtime_dispatch.py lines 199–238 — _execute_llm closes the shared skill_resolver inside the finally block of the inner try that wraps agent.process_message(). If factory.acreate_agent() fails — for example because a skills: reference is invalid or cannot be resolved — that inner try is never entered, so await skill_resolver.close_all() is never executed. Any RegistryClient / httpx.AsyncClient pools owned by the resolver are then leaked.

   The other three dispatch paths (_execute_graph, _execute_graph_stream, _execute_llm_stream) all close the resolver in a finally that runs even when agent creation fails; _execute_llm should be consistent with them.

   Recommendation: Restructure _execute_llm so that skill_resolver.close_all() is awaited in a finally covering the agent-creation call (or otherwise guarantee cleanup runs before the function returns on any exception path).

Minor Issues

1. src/cleveractors/runtime_dispatch.py _build_skill_loader lines 80–81 — The shared PackageContentResolver is only created when executor.local_store is set. When no local store is configured, AgentFactory falls back to the default SkillLoader(), which constructs and tears down an ad-hoc resolver for every skill reference. This still works for registry: references, but it defeats the sharing/cleanup intent for actors that list multiple registry-only skills.

   Recommendation: Always build and return the shared PackageContentResolver (with or without a local_store) so that every reference in a skills: list shares one resolver and one cleanup path.

Nits

None

Summary

This PR delivers a clean, registry-agnostic skill-loading architecture that aligns well with ADR-2034 and reuses the existing package-resolution machinery. The schema validation, progressive-disclosure model, async resolution path, and tool-agent integration are all solid. Once the single-LLM cleanup gap is fixed, this should be ready to merge.

## PR Review: !94 (Ticket #88) ### Verdict: Request Changes The implementation is functionally complete and addresses the blockers from the prior review rounds: the async resolution path now makes `registry:` skill references work from the production runtime dispatch, duplicate skill names are rejected, the loaded skill catalogue is forwarded through the stuck-model synthesis follow-up, and base64/binary resources are handled safely. The test suite is extensive and the Robot integration test gives good end-to-end confidence. I did not re-raise the `docs/index.md` §4.4 update or the coverage-threshold discussion, since you have explicitly treated those as deferred/out-of-scope in your follow-up comments. There is one remaining error-path cleanup bug in the single-LLM dispatch path that should be fixed before merge. ### Critical Issues None ### Major Issues 1. **`src/cleveractors/runtime_dispatch.py` lines 199–238** — `_execute_llm` closes the shared `skill_resolver` inside the `finally` block of the inner `try` that wraps `agent.process_message()`. If `factory.acreate_agent()` fails — for example because a `skills:` reference is invalid or cannot be resolved — that inner `try` is never entered, so `await skill_resolver.close_all()` is never executed. Any `RegistryClient` / `httpx.AsyncClient` pools owned by the resolver are then leaked. The other three dispatch paths (`_execute_graph`, `_execute_graph_stream`, `_execute_llm_stream`) all close the resolver in a `finally` that runs even when agent creation fails; `_execute_llm` should be consistent with them. **Recommendation:** Restructure `_execute_llm` so that `skill_resolver.close_all()` is awaited in a `finally` covering the agent-creation call (or otherwise guarantee cleanup runs before the function returns on any exception path). ### Minor Issues 1. **`src/cleveractors/runtime_dispatch.py` `_build_skill_loader` lines 80–81** — The shared `PackageContentResolver` is only created when `executor.local_store` is set. When no local store is configured, `AgentFactory` falls back to the default `SkillLoader()`, which constructs and tears down an ad-hoc resolver for every skill reference. This still works for `registry:` references, but it defeats the sharing/cleanup intent for actors that list multiple registry-only skills. **Recommendation:** Always build and return the shared `PackageContentResolver` (with or without a `local_store`) so that every reference in a `skills:` list shares one resolver and one cleanup path. ### Nits None ### Summary This PR delivers a clean, registry-agnostic skill-loading architecture that aligns well with ADR-2034 and reuses the existing package-resolution machinery. The schema validation, progressive-disclosure model, async resolution path, and tool-agent integration are all solid. Once the single-LLM cleanup gap is fixed, this should be ready to merge.

CoreRasurae force-pushed feature/skill-package-support from 3e33656c3f to 67ffee3475 2026-08-03 15:44:03 +00:00 Compare

CoreRasurae force-pushed feature/skill-package-support from 67ffee3475 to ac410de3be 2026-08-03 15:44:54 +00:00 Compare

CoreRasurae commented 2026-08-03 15:54:31 +00:00

Author

Member

Copy link

The implementation was modified to also deal with the download of private skill packages that need authentication, as well as also supporting skills within the interactive mode (streaming mode). This all part of the same issue, the supporting skills, requires all this together.

The implementation was modified to also deal with the download of private skill packages that need authentication, as well as also supporting skills within the interactive mode (streaming mode). This all part of the same issue, the supporting skills, requires all this together.

CoreRasurae force-pushed feature/skill-package-support from ac410de3be to bcd1ad0dac 2026-08-03 16:05:28 +00:00 Compare

CoreRasurae commented 2026-08-03 16:07:34 +00:00

Author

Member

Copy link

Addressed the outstanding review items from the 2026-08-03 14:53 review (commit bcd1ad0):

Fixed:

1. Major — _execute_llm (src/cleveractors/runtime_dispatch.py) now closes skill_resolver in a finally that wraps the factory.acreate_agent() call itself, instead of only the finally that runs after process_message() (which is only reached once agent creation has already succeeded). A failed agent creation — e.g. an invalid skills: reference — now still releases the resolver's RegistryClient/httpx.AsyncClient pools, matching _execute_graph/_execute_llm_stream/_execute_graph_stream, which already closed the resolver on that path.
2. Minor — _build_skill_loader (same file) now always constructs a real PackageContentResolver — with local_store=None/api_key=None when neither is configured — instead of returning (None, None) and letting AgentFactory fall back to its default, storeless SkillLoader(). Every reference in a skills: list now shares one resolver and one cleanup path regardless of whether a local store or registry API key is configured, not only when one of them happens to be set.

Verified both against ADR-2034 and docs/index.md first: neither fix touches or conflicts with any normative behaviour, they're both implementation-only bugs in runtime_dispatch.py.

Added 2 new Behave scenarios covering both regressions directly (an actor whose skill fails to load still closes the resolver; _build_skill_loader returns a real loader/resolver pair with no local store or registry API key configured) — 97 scenarios now in features/agent_skills.feature, all passing. Full nox suite green: lint, typecheck, and targeted unit_tests runs covering agent_skills.feature plus the other runtime-dispatch-adjacent feature files (runtime_dispatch_coverage, runtime_coverage, runtime_extended_coverage, actor_result_token_counting, credential_injection, execute_stream, tool_agent_class_injection — 466 scenarios, 0 failed).

Not changed: the public build_skill_loader() in agents/skills.py still returns (None, None) when neither local_store nor api_key is given — that contract has its own dedicated Behave scenarios and is unrelated to the leak; I only changed the private runtime_dispatch._build_skill_loader() wrapper used by the four dispatch functions, which is exactly what your comment pointed at.

As before, the docs/index.md §4.4 update and the coverage-threshold discussion remain untouched, since you've already indicated you're treating those as deferred/out-of-scope and didn't re-raise them in this round.

Addressed the outstanding review items from the 2026-08-03 14:53 review (commit bcd1ad0): **Fixed:** 1. **Major** — `_execute_llm` (`src/cleveractors/runtime_dispatch.py`) now closes `skill_resolver` in a `finally` that wraps the `factory.acreate_agent()` call itself, instead of only the `finally` that runs after `process_message()` (which is only reached once agent creation has already succeeded). A failed agent creation — e.g. an invalid `skills:` reference — now still releases the resolver's `RegistryClient`/`httpx.AsyncClient` pools, matching `_execute_graph`/`_execute_llm_stream`/`_execute_graph_stream`, which already closed the resolver on that path. 2. **Minor** — `_build_skill_loader` (same file) now always constructs a real `PackageContentResolver` — with `local_store=None`/`api_key=None` when neither is configured — instead of returning `(None, None)` and letting `AgentFactory` fall back to its default, storeless `SkillLoader()`. Every reference in a `skills:` list now shares one resolver and one cleanup path regardless of whether a local store or registry API key is configured, not only when one of them happens to be set. Verified both against ADR-2034 and `docs/index.md` first: neither fix touches or conflicts with any normative behaviour, they're both implementation-only bugs in `runtime_dispatch.py`. Added 2 new Behave scenarios covering both regressions directly (an actor whose skill fails to load still closes the resolver; `_build_skill_loader` returns a real loader/resolver pair with no local store or registry API key configured) — 97 scenarios now in `features/agent_skills.feature`, all passing. Full `nox` suite green: `lint`, `typecheck`, and targeted `unit_tests` runs covering `agent_skills.feature` plus the other runtime-dispatch-adjacent feature files (`runtime_dispatch_coverage`, `runtime_coverage`, `runtime_extended_coverage`, `actor_result_token_counting`, `credential_injection`, `execute_stream`, `tool_agent_class_injection` — 466 scenarios, 0 failed). **Not changed:** the public `build_skill_loader()` in `agents/skills.py` still returns `(None, None)` when neither `local_store` nor `api_key` is given — that contract has its own dedicated Behave scenarios and is unrelated to the leak; I only changed the private `runtime_dispatch._build_skill_loader()` wrapper used by the four dispatch functions, which is exactly what your comment pointed at. As before, the `docs/index.md` §4.4 update and the coverage-threshold discussion remain untouched, since you've already indicated you're treating those as deferred/out-of-scope and didn't re-raise them in this round.

hurui200320 requested changes 2026-08-03 16:27:19 +00:00

Dismissed

hurui200320 left a comment

Copy link

PR Review: !94 (Ticket #88)

Verdict: Request Changes

The implementation is functionally complete and the blockers from the previous review round (skill_resolver cleanup on failed agent creation in _execute_llm, and _build_skill_loader always constructing a shared resolver) are both fixed in commit bcd1ad0. The async resolution path, duplicate-skill-name rejection, skill-tool catalogue forwarding through stuck-model synthesis, base64/binary resource handling, and registry credential wiring are all solid.

However, one new functional issue requires a fix before approval: SkillValidator does not correctly handle the registry response's package_id field, so a skill resolved from a registry: reference gets a different content-addressed identity than the same skill resolved from local: or ID:. There are also two smaller robustness/coverage observations.

I have not re-raised the docs/index.md §4.4 documentation update or the noxfile.py coverage-threshold discussion, as you have explicitly treated those as deferred/out-of-scope.

Critical Issues

None

Major Issues

1. src/cleveractors/agents/skill_schema.py lines 21–32 and 331–336 — _RESOLVER_METADATA_KEYS does not include "package_id", and _resolve_package_id() only checks "_package_id". When a skill is resolved from a registry: reference, PackageContentResolver._resolve_registry_async() merges the registry response (which includes package_id and type per RegistryClient.resolve_package) into the content dict. Because package_id is neither used as the authoritative ID nor stripped before canonicalization, SkillValidator computes the skill's pkg_skl_ SHA-1 over content that includes the registry-specific package_id field. The same skill resolved from local: (where _local_package_result stores the ID as "_package_id", which is stripped) will produce a different Package ID, violating the content-addressing invariant in Package Registry Standard §5/§6 and ADR-2034 D-3.

   Recommendation: Treat the registry's package_id as authoritative: in _resolve_package_id(), check both content.get("_package_id") and content.get("package_id"). Also add "package_id" to _RESOLVER_METADATA_KEYS so that any remaining package_id field does not pollute the canonical form if the code falls through to compute_package_id(). Add a Behave/Robot assertion that the same skill content resolved via registry: and local: yields the same Skill.package_id.

Minor Issues

1. src/cleveractors/agents/skill_resolution.py lines 119–129 — _require_resolved() returns the _IdReferenceStrategy placeholder dict ({"id": ..., "type": "id", ...}) as if it were valid content. When an ID: skill reference cannot be resolved against the local store's identity map, SkillValidator.validate() then fails with "missing the required discriminator field 'skill: true'" instead of a clear package-not-found error.

   Recommendation: Detect the placeholder in SkillReferenceResolver._require_resolved() (e.g. resolved.get("type") == "id") and raise PackageNotFoundError with a message naming the unresolved ID: reference.

2. src/cleveractors/agents/skills.py lines 278–288 — _build_tools() only deduplicates a pre-declared skill tool when it appears as a string "skill" or as a dict with a top-level "name" key. If a user supplies the already-normalized OpenAI format {"type": "function", "function": {"name": "skill"}}, the check misses it and a second skill tool is appended.

   Recommendation: Extend the deduplication check to recognize OpenAI-formatted tool dicts, or normalize each entry with normalize_tool_entry() before checking for the skill tool.

Nits

None

Summary

This PR delivers a clean, registry-agnostic skill-loading architecture that aligns well with ADR-2034. The schema validation, progressive-disclosure model, async resolution path, tool-agent integration, and previous review blockers are all addressed. Once the package_id canonicalization bug is fixed (and the two minor robustness items are considered), this should be ready to merge.

## PR Review: !94 (Ticket #88) ### Verdict: Request Changes The implementation is functionally complete and the blockers from the previous review round (skill_resolver cleanup on failed agent creation in `_execute_llm`, and `_build_skill_loader` always constructing a shared resolver) are both fixed in commit `bcd1ad0`. The async resolution path, duplicate-skill-name rejection, skill-tool catalogue forwarding through stuck-model synthesis, base64/binary resource handling, and registry credential wiring are all solid. However, one new functional issue requires a fix before approval: `SkillValidator` does not correctly handle the registry response's `package_id` field, so a skill resolved from a `registry:` reference gets a different content-addressed identity than the same skill resolved from `local:` or `ID:`. There are also two smaller robustness/coverage observations. I have not re-raised the `docs/index.md` §4.4 documentation update or the `noxfile.py` coverage-threshold discussion, as you have explicitly treated those as deferred/out-of-scope. ### Critical Issues None ### Major Issues 1. **`src/cleveractors/agents/skill_schema.py` lines 21–32 and 331–336** — `_RESOLVER_METADATA_KEYS` does not include `"package_id"`, and `_resolve_package_id()` only checks `"_package_id"`. When a skill is resolved from a `registry:` reference, `PackageContentResolver._resolve_registry_async()` merges the registry response (which includes `package_id` and `type` per `RegistryClient.resolve_package`) into the content dict. Because `package_id` is neither used as the authoritative ID nor stripped before canonicalization, `SkillValidator` computes the skill's `pkg_skl_` SHA-1 over content that includes the registry-specific `package_id` field. The same skill resolved from `local:` (where `_local_package_result` stores the ID as `"_package_id"`, which is stripped) will produce a different Package ID, violating the content-addressing invariant in Package Registry Standard §5/§6 and ADR-2034 D-3. **Recommendation:** Treat the registry's `package_id` as authoritative: in `_resolve_package_id()`, check both `content.get("_package_id")` and `content.get("package_id")`. Also add `"package_id"` to `_RESOLVER_METADATA_KEYS` so that any remaining `package_id` field does not pollute the canonical form if the code falls through to `compute_package_id()`. Add a Behave/Robot assertion that the same skill content resolved via `registry:` and `local:` yields the same `Skill.package_id`. ### Minor Issues 1. **`src/cleveractors/agents/skill_resolution.py` lines 119–129** — `_require_resolved()` returns the `_IdReferenceStrategy` placeholder dict (`{"id": ..., "type": "id", ...}`) as if it were valid content. When an `ID:` skill reference cannot be resolved against the local store's identity map, `SkillValidator.validate()` then fails with "missing the required discriminator field 'skill: true'" instead of a clear package-not-found error. **Recommendation:** Detect the placeholder in `SkillReferenceResolver._require_resolved()` (e.g. `resolved.get("type") == "id"`) and raise `PackageNotFoundError` with a message naming the unresolved `ID:` reference. 2. **`src/cleveractors/agents/skills.py` lines 278–288** — `_build_tools()` only deduplicates a pre-declared `skill` tool when it appears as a string `"skill"` or as a dict with a top-level `"name"` key. If a user supplies the already-normalized OpenAI format `{"type": "function", "function": {"name": "skill"}}`, the check misses it and a second `skill` tool is appended. **Recommendation:** Extend the deduplication check to recognize OpenAI-formatted tool dicts, or normalize each entry with `normalize_tool_entry()` before checking for the skill tool. ### Nits None ### Summary This PR delivers a clean, registry-agnostic skill-loading architecture that aligns well with ADR-2034. The schema validation, progressive-disclosure model, async resolution path, tool-agent integration, and previous review blockers are all addressed. Once the `package_id` canonicalization bug is fixed (and the two minor robustness items are considered), this should be ready to merge.

CoreRasurae force-pushed feature/skill-package-support from bcd1ad0dac to bff76e8e9e 2026-08-03 18:17:23 +00:00 Compare

CoreRasurae commented 2026-08-03 18:17:56 +00:00

Author

Member

Copy link

Addressed the outstanding review items from the 2026-08-03 16:27 review (commit bcd1ad0), now in commit bff76e8:

Fixed:

1. Major — SkillValidator._resolve_package_id() (skill_schema.py) now also checks a bare package_id key (added to _RESOLVER_METADATA_KEYS too), so a skill resolved via registry: computes/reuses the same content-addressed package_id as the identical skill resolved via local:/ID:, per §16.2/ADR-2034 D-3. Added a Behave scenario asserting a local-shaped and a registry-shaped resolution of identical schema fields agree on package_id.
2. Minor — SkillReferenceResolver._require_resolved() (skill_resolution.py) now also rejects _IdReferenceStrategy's not-found placeholder ({"type": "id", ...}), which previously slipped past the is None check and surfaced as a confusing "missing discriminator field 'skill: true'" validation error instead of a clear PackageNotFoundError, per D-7. Added sync + async Behave scenarios for an unresolvable ID: reference.
3. Minor — SkillLoader._build_tools() (skills.py) now also recognizes an already-declared skill tool in normalized OpenAI function-calling form ({"type": "function", "function": {"name": "skill"}}), not just the bare {"name": "skill"} shorthand, so it's never duplicated. Added a Behave scenario for this shape.

Verified all three against docs/actor-registry-standard.md §16.2 and ADR-2034 D-3/D-7 first — none of these fixes touch or conflict with any normative behaviour.

features/agent_skills.feature now has 101 scenarios (4 new), all passing. nox -s lint and nox -s typecheck clean; targeted nox -s unit_tests -- features/agent_skills.feature and nox -s coverage_report -- features/agent_skills.feature show the three touched files (skill_schema.py, skills.py at 100%, skill_resolution.py at 93% with only pre-existing, unrelated defensive-logging lines uncovered).

As before, the docs/index.md §4.4 update and the coverage-threshold discussion remain untouched, since you've indicated you're treating those as deferred/out-of-scope.

Addressed the outstanding review items from the 2026-08-03 16:27 review (commit bcd1ad0), now in commit bff76e8: **Fixed:** 1. **Major** — `SkillValidator._resolve_package_id()` (`skill_schema.py`) now also checks a bare `package_id` key (added to `_RESOLVER_METADATA_KEYS` too), so a skill resolved via `registry:` computes/reuses the same content-addressed `package_id` as the identical skill resolved via `local:`/`ID:`, per §16.2/ADR-2034 D-3. Added a Behave scenario asserting a local-shaped and a registry-shaped resolution of identical schema fields agree on `package_id`. 2. **Minor** — `SkillReferenceResolver._require_resolved()` (`skill_resolution.py`) now also rejects `_IdReferenceStrategy`'s not-found placeholder (`{"type": "id", ...}`), which previously slipped past the `is None` check and surfaced as a confusing "missing discriminator field 'skill: true'" validation error instead of a clear `PackageNotFoundError`, per D-7. Added sync + async Behave scenarios for an unresolvable `ID:` reference. 3. **Minor** — `SkillLoader._build_tools()` (`skills.py`) now also recognizes an already-declared `skill` tool in normalized OpenAI function-calling form (`{"type": "function", "function": {"name": "skill"}}`), not just the bare `{"name": "skill"}` shorthand, so it's never duplicated. Added a Behave scenario for this shape. Verified all three against `docs/actor-registry-standard.md` §16.2 and ADR-2034 D-3/D-7 first — none of these fixes touch or conflict with any normative behaviour. `features/agent_skills.feature` now has 101 scenarios (4 new), all passing. `nox -s lint` and `nox -s typecheck` clean; targeted `nox -s unit_tests -- features/agent_skills.feature` and `nox -s coverage_report -- features/agent_skills.feature` show the three touched files (`skill_schema.py`, `skills.py` at 100%, `skill_resolution.py` at 93% with only pre-existing, unrelated defensive-logging lines uncovered). As before, the `docs/index.md` §4.4 update and the coverage-threshold discussion remain untouched, since you've indicated you're treating those as deferred/out-of-scope.

hurui200320 approved these changes 2026-08-04 04:53:37 +00:00

hurui200320 left a comment

Copy link

PR Review: !94 (Ticket #88)

Verdict: Approve

All three items raised in the previous review round (2026-08-03 16:27, review id 9773) are fixed in commit bff76e8, and the fixes are backed by targeted Behave/Robot coverage. No new critical or major issues were introduced. I did not re-raise the deferred/out-of-scope items (docs/index.md §4.4 update and the coverage-threshold discussion), per the prior agreement in the PR thread.

Critical Issues

None

Major Issues

None

Minor Issues

None

Nits

None

Summary

This PR delivers the normative Skill package schema and agent-side skill-loading support described in ADR-2034. The previous review round asked for three specific fixes:

1. package_id canonicalization for registry: responses — SkillValidator._resolve_package_id() in src/cleveractors/agents/skill_schema.py now treats both _package_id (local/ID convention) and bare package_id (registry response) as authoritative, and "package_id" was added to _RESOLVER_METADATA_KEYS so it does not pollute the canonical form. A Behave scenario asserts that local-shaped and registry-shaped resolutions of identical content yield the same package_id.

2. Clear error for unresolved ID: references — SkillReferenceResolver._require_resolved() in src/cleveractors/agents/skill_resolution.py now rejects _IdReferenceStrategy's {"type": "id", ...} placeholder and raises PackageNotFoundError instead of letting it reach validation as a confusing "missing discriminator field" error. Both sync and async paths have coverage.

3. OpenAI-format skill tool deduplication — SkillLoader._build_tools() in src/cleveractors/agents/skills.py now recognizes an already-declared skill tool in normalized OpenAI function-calling form ({"type": "function", "function": {"name": "skill"}}) in addition to the shorthand forms, preventing duplicate tool entries.

Earlier blockers also remain resolved: the stuck-model synthesis path forwards _loaded_skills via _build_tool_context(), duplicate skill names raise AgentCreationError, the async resolution path (aresolve/aload/acreate_agent) makes registry: references work from the runtime dispatch, the single-LLM dispatch closes the shared skill resolver even when agent creation fails, _build_skill_loader() always constructs a shared resolver, and base64/binary resources are handled safely.

The test suite is extensive: 101 Behave scenarios in features/agent_skills.feature, a stuck-model synthesis regression scenario, plus Robot integration tests for local-skill resolution and registry authentication. The implementation reuses the existing registry resolution machinery, keeps LLMAgent registry-agnostic, and aligns with the agentskills.io format. This is ready to merge.

## PR Review: !94 (Ticket #88) ### Verdict: Approve All three items raised in the previous review round (2026-08-03 16:27, review id 9773) are fixed in commit `bff76e8`, and the fixes are backed by targeted Behave/Robot coverage. No new critical or major issues were introduced. I did not re-raise the deferred/out-of-scope items (`docs/index.md` §4.4 update and the coverage-threshold discussion), per the prior agreement in the PR thread. ### Critical Issues None ### Major Issues None ### Minor Issues None ### Nits None ### Summary This PR delivers the normative Skill package schema and agent-side skill-loading support described in ADR-2034. The previous review round asked for three specific fixes: 1. **`package_id` canonicalization for `registry:` responses** — `SkillValidator._resolve_package_id()` in `src/cleveractors/agents/skill_schema.py` now treats both `_package_id` (local/ID convention) and bare `package_id` (registry response) as authoritative, and `"package_id"` was added to `_RESOLVER_METADATA_KEYS` so it does not pollute the canonical form. A Behave scenario asserts that local-shaped and registry-shaped resolutions of identical content yield the same `package_id`. 2. **Clear error for unresolved `ID:` references** — `SkillReferenceResolver._require_resolved()` in `src/cleveractors/agents/skill_resolution.py` now rejects `_IdReferenceStrategy`'s `{"type": "id", ...}` placeholder and raises `PackageNotFoundError` instead of letting it reach validation as a confusing "missing discriminator field" error. Both sync and async paths have coverage. 3. **OpenAI-format `skill` tool deduplication** — `SkillLoader._build_tools()` in `src/cleveractors/agents/skills.py` now recognizes an already-declared `skill` tool in normalized OpenAI function-calling form (`{"type": "function", "function": {"name": "skill"}}`) in addition to the shorthand forms, preventing duplicate tool entries. Earlier blockers also remain resolved: the stuck-model synthesis path forwards `_loaded_skills` via `_build_tool_context()`, duplicate skill names raise `AgentCreationError`, the async resolution path (`aresolve`/`aload`/`acreate_agent`) makes `registry:` references work from the runtime dispatch, the single-LLM dispatch closes the shared skill resolver even when agent creation fails, `_build_skill_loader()` always constructs a shared resolver, and base64/binary resources are handled safely. The test suite is extensive: 101 Behave scenarios in `features/agent_skills.feature`, a stuck-model synthesis regression scenario, plus Robot integration tests for local-skill resolution and registry authentication. The implementation reuses the existing registry resolution machinery, keeps `LLMAgent` registry-agnostic, and aligns with the agentskills.io format. This is ready to merge.

CoreRasurae force-pushed feature/skill-package-support from bff76e8e9e to 57329ee52e 2026-08-04 06:11:42 +00:00 Compare

CoreRasurae added 1 commit 2026-08-04 06:14:56 +00:00

feat(skills): add skill package schema and agent-side skill loading support ... 882336c478

Extends the Package Registry Standard with a normative Skill package
schema (§16) aligned with the open agentskills.io Agent Skills format,
and adds an optional `skills` field to `type: llm` agent configs so an
Actor can attach one or more resolved Skill packages to an agent.

SkillLoader (new) resolves each `skills` reference through the existing
cleveractors.registry client (registry:/ID:/local: schemes), mirroring
the established template package-reference resolution pattern in
cleveractors.templates.base._resolve_package_ref rather than
introducing a parallel resolution path. Resolution is available in both
sync (SkillReferenceResolver.resolve/SkillLoader.load) and async
(.aresolve/.aload) forms; AgentFactory exposes a matching
acreate_agent()/_acreate_agent_instance() pair so the async runtime
dispatch layer (_execute_llm/_execute_graph/etc.) resolves `skills`
through PackageContentResolver.aresolve() instead of nesting
asyncio.run() inside its own already-running event loop — the
`registry:` scheme now resolves correctly regardless of whether agent
creation happens from plain sync code or from async runtime dispatch.
Every skill reference in a `skills` list shares one
PackageContentResolver built unconditionally per agent-creation call
(regardless of whether a local store or registry API key is
configured) instead of building one ad hoc per reference, and its
connections are closed once agent creation for that request completes
— including when agent creation itself fails, e.g. because of an
invalid `skills` reference — alongside the existing per-agent
cleanup(). SkillValidator (new) checks resolved content against the
D-2 schema (name/description constraints lifted from agentskills.io,
resource path/encoding rules — including that base64-encoded resource
content actually decodes as well-formed base64 — and uniqueness of
skill names within a single `skills` list) and computes a
content-addressed `package_id` via the existing Canonicalizer that is
identical regardless of which reference scheme resolved the content: a
registry: response's bare `package_id` field and a local:/ID:
resolution's `_package_id` field are both recognized as authoritative
and excluded from the canonicalized form. A reference that cannot be
resolved — including an `ID:` reference absent from the local store's
identity map — now surfaces as a clear PackageNotFoundError rather than
a confusing schema-validation error.

Resolved skills are disclosed to the model in three stages, per D-4:
discovery (name+description folded into system_prompt), activation (a
synthesized `skill` tool call returns full instructions — recognizing
an already-declared `skill` tool in either shorthand (`{"name":
"skill"}`) or normalized OpenAI function-calling form, so it is never
duplicated), and execution (a further call reads a bundled resource,
reusing file_read's offset/max_chars pagination convention from
ADR-2033, and decoding base64 resources safely even when the underlying
bytes are not valid UTF-8). All registry awareness lives in SkillLoader
at the factory layer; LLMAgent only gains a `_skills` value forwarded
through its existing tool-call context — including the ephemeral
ToolAgent spawned by the stuck-model synthesis follow-up round — and
ToolAgent gains one new built-in tool handler; neither imports anything
from cleveractors.registry. Skill.to_context_dict() surfaces the full
set of optional schema fields (license, compatibility, metadata,
allowed_tools) alongside name/description/instructions/resources.

The `skills` field itself is documented only in ADR-2034, not mirrored
into docs/index.md, matching how ADR-2031/ADR-2032 handled their own
new LLM agent config fields (graduation into the normative spec is
deferred to a future ADR). The Skill package schema, which has no other
home, is appended as a new §16 in docs/actor-registry-standard.md
(after the original §1-15 body, with a version bump to 1.1.0) rather
than spliced into the existing §3.2 package-type table.

Behave scenarios cover schema validation, all three reference
schemes in both their sync and async forms (including package_id parity
between registry: and local:-shaped resolutions, and an unresolvable
ID: reference), loader orchestration (including tool-declaration
dedup in both supported shapes), factory/agent integration (including
the async factory path and an end-to-end run through the real
Executor/runtime dispatch layer with a real on-disk skill package), and
the ToolAgent skill tool (activation, resource reads, pagination, error
paths). A Robot Framework suite exercises the same flow against a real
on-disk skill file and real LocalPackageStore/PackageContentResolver
resolution end to end, mocking only the LLM API call. Full nox suite
green.

ISSUES CLOSED: #88

CoreRasurae force-pushed feature/skill-package-support from 57329ee52e to 882336c478 2026-08-04 06:14:56 +00:00 Compare

CoreRasurae commented 2026-08-04 10:05:02 +00:00

Author

Member

Copy link

PR has already been merged. This is a bug in Gitea.

PR has already been merged. This is a bug in Gitea.

CoreRasurae closed this pull request 2026-08-04 10:05:17 +00:00

Some checks failed

Hide all checks

CI / lint (pull_request) Successful in 1m6s

Details

CI / typecheck (pull_request) Successful in 1m56s

Details

CI / quality (pull_request) Successful in 1m5s

Details

CI / security (pull_request) Successful in 1m27s

Details

CI / build (pull_request) Successful in 58s

Details

CI / unit_tests (pull_request) Successful in 4m34s

Details

CI / coverage (pull_request) Successful in 4m51s

Details

CI / benchmark (pull_request) Failing after 33m57s

Details

CI / integration_tests (pull_request) Successful in 3m37s

Details

CI / status-check (pull_request) Successful in 18s

Details

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

Reviewers

No reviewers

hurui200320

Labels

Clear labels

  

auto/blocked-by-deps

PR blocked by an open issue dependency. Operator must close the dep (or remove the dependency link) before the merge driver can act. Auto-cleared by merge_drive when no open deps remain.

  

auto/ci-timeout

Most recent merge cycle hit CI timeout. Driver excludes this PR while last merge_cycle row is < 30 min old; label persists thereafter as visible history.

  

auto/claimed-implementer

Currently being processed by an implementer worker.

  

auto/claimed-merge

Currently being processed by the merge driver.

  

auto/claimed-reviewer

Currently being processed by a reviewer worker.

  

auto/driver-down

Merge driver heartbeat stale; pipeline halted. Closed automatically on next clean tick.

  

auto/invariant-violation

Detected master commit violating the strict merge invariant. Tracked as an issue (not a PR label); kept here for label completeness.

  

auto/last-attempt-tier-0

In-cycle escalation: most recent attempt ran at the Tier 0 slot (`tier-0`). Slot's model defined in .opencode/models/tiers.yaml.

  

auto/last-attempt-tier-1

In-cycle escalation: most recent attempt ran at the Tier 1 slot (`tier-1`). Slot's model defined in .opencode/models/tiers.yaml.

  

auto/last-attempt-tier-2

In-cycle escalation: most recent attempt ran at the Tier 2 slot (`tier-2`). Slot's model defined in .opencode/models/tiers.yaml. Gated behind IMPLEMENTER_ESCALATION_TIER2_ENABLED.

  

auto/last-attempt-tier-min

In-cycle escalation: most recent attempt ran at the Tier -1 slot (`tier-min`). Slot's model defined in .opencode/models/tiers.yaml. Suffix is ``-min`` (not ``--1``) so the Forgejo UI reads naturally.

  

Automation Tracking

Tracking issues used by the AI Automation system for agents to communicate and report.

  

auto/needs-conflict-resolution

Rebase conflict needs LLM conflict-resolver.

  

auto/needs-implementer

Failing CI needs implementer attention.

  

auto/postmortem

Documenting a driver incident or rollback.

  

auto/ready-to-merge

Reviewer has APPROVED this PR and no later REQUEST_CHANGES is outstanding. The merge driver requires this label to even consider a PR for merging. Set by the reviewer worker on APPROVE; cleared on REQUEST_CHANGES.

  

auto/restart-throttled

Train repeatedly lost master-tempo races. Driver excludes via merge_cycle until cooldown elapses; label persists as visible history.

  

auto/revert

Revert PR backing out an invariant violation. Fast-tracked through the merge driver.

  

auto/sentinel

Sentinel PR duplicated from upstream into a personal fork by tools/duplicate_prs_to_fork.py for pipeline testing. Lives only in the fork; the canonical pipeline never sees it.

  

auto/stale-inactivity

No implementer activity for N days. Flagged for human review. Auto-cleared on next push to head branch.

  

auto/unstable

Repeatedly fails on current master (>= 3 ci-fail-on-rebased-sha releases in 12 h). Excluded from driver until human triage.

  

Blocked

A ticket in a blocked state and unable to complete until some other task is completed first.

  

Bounty

$100

A bounty of $100 for any open-source contributor who provides a MR that solves this issue

  

Bounty

$1000

A bounty of $1000 for any open-source contributor who provides a MR that solves this issue

  

Bounty

$10000

A bounty of $10000 for any open-source contributor who provides a MR that solves this issue

  

Bounty

$20

A bounty of $20 for any open-source contributor who provides a MR that solves this issue

  

Bounty

$2000

A bounty of $2000 for any open-source contributor who provides a MR that solves this issue

  

Bounty

$250

A bounty of $250 for any open-source contributor who provides a MR that solves this issue

  

Bounty

$50

A bounty of $50 for any open-source contributor who provides a MR that solves this issue

  

Bounty

$500

A bounty of $500 for any open-source contributor who provides a MR that solves this issue

  

Bounty

$5000

A bounty of $5000 for any open-source contributor who provides a MR that solves this issue

  

Bounty

$750

A bounty of $750 for any open-source contributor who provides a MR that solves this issue

  

MoSCoW

Could have

Could have feature in order to satisfy the epic/legendary.

  

MoSCoW

Must have

Must have feature in order to satisfy the epic/legendary.

  

MoSCoW

Should have

Should have feature in order to satisfy the epic/legendary.

  

Needs Feedback

There are questions in the ticket that can not be completed until the project owner provides clarity.

  

Points

1

1 man-hours worth of work for an expert with no learning curve.

  

Points

13

13 man-hours worth of work for an expert with no learning curve.

  

Points

2

2 man-hours worth of work for an expert with no learning curve.

  

Points

21

21 man-hours worth of work for an expert with no learning curve.

  

Points

3

3 man-hours worth of work for an expert with no learning curve.

  

Points

34

34 man-hours worth of work for an expert with no learning curve.

  

Points

5

5 man-hours worth of work for an expert with no learning curve.

  

Points

55

55 man-hours worth of work for an expert with no learning curve.

  

Points

8

8 man-hours worth of work for an expert with no learning curve.

  

Points

88

88 man-hours worth of work for an expert with no learning curve.

  

Priority

Backlog

This ticket has backlogged priority and is not to be worked on yet

  

Priority

CI Blocker

Critical priority issue that blocks CI/CD pipeline and prevents PR merges

  

Priority

Critical

The priority is critical

  

Priority

High

The priority is high

  

Priority

Low

The priority is low

  

Priority

Medium

The priority is medium

  

Signed-off: Owner

When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.

  

Signed-off: Scrum Master

When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.

  

Signed-off: Tech Lead

When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.

  

Spike

A ticket for learning a tool or technology that is needed to be able to do future planning and design.

  

State

Completed

The ticket has been fully implemented, completed, and merged with the source code. This label should only be applied once a ticket is closed.

  

State

Duplicate

A ticket that represents the same content as an existing ticket.

  

State

In Progress

A ticket that is actively being developed.

  

State

In Review

A ticket that has had some code completed to implement but is waiting to pass peer review and is not yet merged in.

  

State

Paused

This ticket's work started but wasn't finished. It's on hold (likely in a feature branch) and will be resumed later, either due to a blocker or a delay.

  

State

Unverified

All new tickets start in this state. A developer may set it to show the ticket is unverified. This means we haven't agreed to work on it. It will either move to a verified state or be closed as wontdo.

  

State

Verified

The issue has been verified by a developer as legitimate. It will be worked on and verified tickets are now considered part of the backlog.

  

State

Wont Do

This ticket has been decided it wont be done. This may mean the bug has been determined to not be real (cant verify) or the feature is one we have decided we dont want to adopt.

  

Type

Automation

Any edits or discussion about the AI automated coding system.

  

Type

Bug

Something that doesnt work as intended.

  

Type

Discussion

Anytime a ticket represents a discussion about a subject and doesnt fall into one of the other categories.

  

Type

Documentation

An error or improvement needed in the documentation.

  

Type

Epic

Any first tier epic. That is, an epic which contains only issues as children and will not have sub-epics.

  

Type

Feature

Some new functionality not present.

  

Type

Legendary

A type of Epic which will contain other Epics.

  

Type

Refactor

A code change that restructures existing code without changing its external behavior.

  

Type

Support

Someone needs help using the project.

  

Type

Task

A generic task that doesnt fit into the other type categories.

  

Type

Testing

Work exclusively focusing on fixing or expanding testing.

No labels

auto/blocked-by-deps

auto/ci-timeout

auto/claimed-implementer

auto/claimed-merge

auto/claimed-reviewer

auto/driver-down

auto/invariant-violation

auto/last-attempt-tier-0

auto/last-attempt-tier-1

auto/last-attempt-tier-2

auto/last-attempt-tier-min

Automation Tracking

auto/needs-conflict-resolution

auto/needs-implementer

auto/postmortem

auto/ready-to-merge

auto/restart-throttled

auto/revert

auto/sentinel

auto/stale-inactivity

auto/unstable

Blocked

Bounty

$100

Bounty

$1000

Bounty

$10000

Bounty

$20

Bounty

$2000

Bounty

$250

Bounty

$50

Bounty

$500

Bounty

$5000

Bounty

$750

MoSCoW

Could have

MoSCoW

Must have

MoSCoW

Should have

Needs Feedback

Points

1

Points

13

Points

2

Points

21

Points

3

Points

34

Points

5

Points

55

Points

8

Points

88

Priority

Backlog

Priority

CI Blocker

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Signed-off: Owner

Signed-off: Scrum Master

Signed-off: Tech Lead

Spike

State

Completed

State

Duplicate

State

In Progress

State

In Review

State

Paused

State

Unverified

State

Verified

State

Wont Do

Type

Automation

Type

Bug

Type

Discussion

Type

Documentation

Type

Epic

Type

Feature

Type

Legendary

Type

Refactor

Type

Support

Type

Task

Type

Testing

Milestone

Clear milestone

No items

No milestone

v2.1.0

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks

#88 feat(skills): add Skill package schema and agent-side skill loading support

cleveragents/cleveractors-core

Reference

cleveragents/cleveractors-core!94

No description provided.