feat(skills): expose assets and gated non-standard resources through the skill tool #167

Open
CoreRasurae wants to merge 1 commit from feature/m1-skill-resource-manifest-and-assets into develop
Member

Summary

Implements the ADR-2034 D-10 read surface for Skill packages:

  • Activation now returns a bundled-resource manifest grouped by references/, assets/, and scripts/, with every entry tagged by its D-8 class.
  • Non-standard top-level folders are listed only when the D-9 double-safety gate is open (global unsafe mode and skill-level safe_mode: false). Reading such a path while either switch is safe fails with an ExecutionError naming both states and the requested path.
  • Assets read exactly like references: UTF-8 content is decoded, genuine binary content is returned as base64, and the existing [SKILL_RESOURCE_READ] envelope plus ADR-2033 pagination are preserved.
  • Script sources remain readable for inspection in every mode; execution is out of scope.
  • Updated the synthesized skill tool schema and the authoring/docs to describe the manifest, asset reads, and gated non-standard access.

Closes #162

Scope

Script execution, execute/args/timeout, and temporary-tree materialization remain the D-10 execution follow-up and are not included here.

Verification

  • nox -s unit_tests — 3202 scenarios passed
  • nox -s integration_tests -- robot/agent_skills.robot — 9 tests passed
  • nox -s coverage_report -- features/agent_skills.feature — new manifest/gate paths fully covered
  • nox -s lint, nox -s format -- --check, nox -s typecheck, nox -s security_scan, nox -s dead_code
  • nox -s docs, nox -s benchmark -- --bench Skill
## Summary Implements the ADR-2034 D-10 read surface for Skill packages: - Activation now returns a bundled-resource manifest grouped by `references/`, `assets/`, and `scripts/`, with every entry tagged by its D-8 class. - Non-standard top-level folders are listed only when the D-9 double-safety gate is open (global unsafe mode **and** skill-level `safe_mode: false`). Reading such a path while either switch is safe fails with an `ExecutionError` naming both states and the requested path. - Assets read exactly like references: UTF-8 content is decoded, genuine binary content is returned as base64, and the existing `[SKILL_RESOURCE_READ]` envelope plus ADR-2033 pagination are preserved. - Script sources remain readable for inspection in every mode; execution is out of scope. - Updated the synthesized `skill` tool schema and the authoring/docs to describe the manifest, asset reads, and gated non-standard access. Closes #162 ## Scope Script execution, `execute`/`args`/`timeout`, and temporary-tree materialization remain the D-10 execution follow-up and are not included here. ## Verification - `nox -s unit_tests` — 3202 scenarios passed - `nox -s integration_tests -- robot/agent_skills.robot` — 9 tests passed - `nox -s coverage_report -- features/agent_skills.feature` — new manifest/gate paths fully covered - `nox -s lint`, `nox -s format -- --check`, `nox -s typecheck`, `nox -s security_scan`, `nox -s dead_code` - `nox -s docs`, `nox -s benchmark -- --bench Skill`
CoreRasurae added this to the v2.1.0 milestone 2026-11-10 14:51:18 +00:00
feat(skills): expose assets and gated non-standard resources through the skill tool
Some checks failed
CI / lint (pull_request) Successful in 56s
CI / typecheck (pull_request) Successful in 1m50s
CI / integration_tests (pull_request) Has started running
CI / security (pull_request) Successful in 1m27s
CI / benchmark (pull_request) Has started running
CI / quality (pull_request) Successful in 2m11s
CI / build (pull_request) Successful in 2m58s
CI / unit_tests (pull_request) Successful in 5m52s
CI / coverage (pull_request) Has started running
CI / status-check (pull_request) Has been cancelled
695cfa7110
Return a class-grouped bundled-resource manifest when a skill activates and gate non-standard folder reads behind the ADR-2034 D-9 double-safety check. Assets become readable like references, with text decoded and binary content returned as base64. The skill tool schema and authoring docs describe the manifest and gating rules; Behave and Robot coverage exercise the manifest, asset reads, and the non-standard read matrix.

ISSUES CLOSED: #162
docs(adr): remove implementation details from ADR-2034
Some checks failed
CI / lint (pull_request) Has started running
CI / security (pull_request) Has started running
CI / quality (pull_request) Has started running
CI / unit_tests (pull_request) Has started running
CI / integration_tests (pull_request) Has started running
CI / build (pull_request) Has started running
CI / benchmark (pull_request) Has started running
CI / coverage (pull_request) Has been cancelled
CI / status-check (pull_request) Has been cancelled
CI / typecheck (pull_request) Has been cancelled
34fc640b40
Describe the Skill package decisions in technology-agnostic terms: remove code identifiers, module paths, test tooling, and implementation tracking while keeping the normative schema, identity, resource taxonomy, and safety-gate decisions.

Refs: #162
CoreRasurae force-pushed feature/m1-skill-resource-manifest-and-assets from 34fc640b40
Some checks failed
CI / lint (pull_request) Has started running
CI / security (pull_request) Has started running
CI / quality (pull_request) Has started running
CI / unit_tests (pull_request) Has started running
CI / integration_tests (pull_request) Has started running
CI / build (pull_request) Has started running
CI / benchmark (pull_request) Has started running
CI / coverage (pull_request) Has been cancelled
CI / status-check (pull_request) Has been cancelled
CI / typecheck (pull_request) Has been cancelled
to ec7a3e3d60
Some checks failed
CI / benchmark (pull_request) Has started running
CI / lint (pull_request) Successful in 1m11s
CI / build (pull_request) Successful in 1m15s
CI / typecheck (pull_request) Successful in 1m33s
CI / quality (pull_request) Successful in 2m11s
CI / security (pull_request) Successful in 2m47s
CI / integration_tests (pull_request) Successful in 4m29s
CI / unit_tests (pull_request) Successful in 6m40s
CI / coverage (pull_request) Failing after 11m11s
CI / status-check (pull_request) Has been cancelled
2026-11-10 15:12:21 +00:00
Compare
hurui200320 left a comment

PR Review: !167 (Ticket #162)

Verdict: Approve

The implementation faithfully delivers the ADR-2034 D-10 read surface scoped by ticket #162: the class-grouped bundled-resource manifest on activation, asset reads with text-decoding/base64 fallback, the D-9 double-safety gate for non-standard reads (correctly refusing when either switch is safe, naming both states and the path), scripts readable in every mode, and the updated skill tool schema plus docs. No critical or major issues were found; the three nits below are optional follow-ups and none block merge.

Critical Issues

None.

Major Issues

None.

Minor Issues

None.

Nits

  1. src/cleveractors/agents/tool.py:1158 — gated paths leak into the "Available resources" error listing. While the D-9 gate is closed, a failed resource lookup lists every resource including non-standard entries — the same paths the activation manifest deliberately hides. ADR-2034 D-10 says the error names "the available resources", so this is compliant as written, but filtering non_standard entries while the gate is closed would be consistent with D-9's hiding intent (a probing read currently reveals gated paths by name). Note docs/tools/built-in-tools.md documents the listing behavior too, so this would need a small ADR/docs amendment if adopted.

  2. src/cleveractors/agents/tool.py:53 — _SKILL_NON_STANDARD_CLASS = "non_standard" duplicates SkillResourceClass.NON_STANDARD.value. If the enum value ever changed, the gate would silently fail open (non-standard reads allowed). The Behave gate matrix uses the real classifier, so drift would be caught by tests, but deriving the constant from the enum (or a parity assertion) is more robust than a string literal.

  3. features/agent_skills.feature:618 — no scenario reads a root-level file. The D-8 fixture set creates LICENSE.txt and the ticket's expected behavior says root-level files remain readable in every mode, but no Behave/Robot scenario reads one. The read path is shared with reference reads so line coverage is unaffected; a one-line read scenario would complete the D-8 read matrix.

Summary

Reviewed against ticket #162 and ADR-2034 (D-8/D-9/D-10), with the Package Registry Standard §16 and the skill docs as supporting context. Verification highlights:

  • Spec compliance: The manifest lists references/, assets/, scripts/ with class tags in ADR-mandated form and hides non-standard entries unless the gate is open; root files are correctly excluded from the manifest and readable in every mode. Non-standard reads raise ExecutionError naming both safety states and the path; the gate predicate (_is_global_unsafe = ToolAgent safe_mode false or context _unsafe_mode) exactly mirrors LLMAgent._resolve_parent_unsafe, and the ephemeral tool agent is constructed with safe_mode = not parent_unsafe (llm.py:1028), so end-to-end semantics are consistent. Script sources remain readable in every mode; execute/args/timeout are correctly absent (deferred to #163, matching the PR's declared scope).
  • Correctness: skill.get("safe_mode", True) is not False is safe given SkillValidator._validate_safe_mode enforces booleans; the manifest is appended to both pagination return branches on activation only; binary base64 resources fall back to the base64 string without crashing, matching the "no binary channel" ADR rationale. The previously-ungated non-standard reads are now intentionally tightened, documented in CHANGELOG and the guide.
  • Tests: New Behave scenarios cover the full gate matrix (refused ×2, allowed ×1), manifest hidden/shown, text/binary asset reads, and script reads, with meaningful assertions (including exact base64 content and negative manifest assertions); Robot covers an asset read and a refused non-standard read end-to-end through the real loader/factory path. before_scenario resets prevent cross-scenario leakage of the skills catalogue, safety flag, and binary fixture.
  • Quality/typing/doc: Code is typed and styled consistently; tool-schema, built-in-tools doc, guide, CHANGELOG, and ADR all match the implemented output format exactly. Commit is atomic with the message matching the issue Metadata; no merge commits.

Prior review comments: none existed at review time, so no previously-raised concerns needed re-checking. Script execution remains tracked by #163 and is deliberately out of scope here.

Nice work — approving. The nits are optional; address them in a follow-up if you agree with any.

## PR Review: !167 (Ticket #162) ### Verdict: Approve The implementation faithfully delivers the ADR-2034 D-10 read surface scoped by ticket #162: the class-grouped bundled-resource manifest on activation, asset reads with text-decoding/base64 fallback, the D-9 double-safety gate for non-standard reads (correctly refusing when either switch is safe, naming both states and the path), scripts readable in every mode, and the updated `skill` tool schema plus docs. No critical or major issues were found; the three nits below are optional follow-ups and none block merge. ### Critical Issues None. ### Major Issues None. ### Minor Issues None. ### Nits 1. **`src/cleveractors/agents/tool.py:1158` — gated paths leak into the "Available resources" error listing.** While the D-9 gate is closed, a failed resource lookup lists every resource including non-standard entries — the same paths the activation manifest deliberately hides. ADR-2034 D-10 says the error names "the available resources", so this is compliant as written, but filtering `non_standard` entries while the gate is closed would be consistent with D-9's hiding intent (a probing read currently reveals gated paths by name). Note `docs/tools/built-in-tools.md` documents the listing behavior too, so this would need a small ADR/docs amendment if adopted. 2. **`src/cleveractors/agents/tool.py:53` — `_SKILL_NON_STANDARD_CLASS = "non_standard"` duplicates `SkillResourceClass.NON_STANDARD.value`.** If the enum value ever changed, the gate would silently fail open (non-standard reads allowed). The Behave gate matrix uses the real classifier, so drift would be caught by tests, but deriving the constant from the enum (or a parity assertion) is more robust than a string literal. 3. **`features/agent_skills.feature:618` — no scenario reads a root-level file.** The D-8 fixture set creates `LICENSE.txt` and the ticket's expected behavior says root-level files remain readable in every mode, but no Behave/Robot scenario reads one. The read path is shared with reference reads so line coverage is unaffected; a one-line read scenario would complete the D-8 read matrix. ### Summary Reviewed against ticket #162 and ADR-2034 (D-8/D-9/D-10), with the Package Registry Standard §16 and the skill docs as supporting context. Verification highlights: - **Spec compliance:** The manifest lists `references/`, `assets/`, `scripts/` with class tags in ADR-mandated form and hides non-standard entries unless the gate is open; root files are correctly excluded from the manifest and readable in every mode. Non-standard reads raise `ExecutionError` naming both safety states and the path; the gate predicate (`_is_global_unsafe` = ToolAgent `safe_mode` false or context `_unsafe_mode`) exactly mirrors `LLMAgent._resolve_parent_unsafe`, and the ephemeral tool agent is constructed with `safe_mode = not parent_unsafe` (llm.py:1028), so end-to-end semantics are consistent. Script sources remain readable in every mode; `execute`/`args`/`timeout` are correctly absent (deferred to #163, matching the PR's declared scope). - **Correctness:** `skill.get("safe_mode", True) is not False` is safe given `SkillValidator._validate_safe_mode` enforces booleans; the manifest is appended to both pagination return branches on activation only; binary base64 resources fall back to the base64 string without crashing, matching the "no binary channel" ADR rationale. The previously-ungated non-standard reads are now intentionally tightened, documented in CHANGELOG and the guide. - **Tests:** New Behave scenarios cover the full gate matrix (refused ×2, allowed ×1), manifest hidden/shown, text/binary asset reads, and script reads, with meaningful assertions (including exact base64 content and negative manifest assertions); Robot covers an asset read and a refused non-standard read end-to-end through the real loader/factory path. `before_scenario` resets prevent cross-scenario leakage of the skills catalogue, safety flag, and binary fixture. - **Quality/typing/doc:** Code is typed and styled consistently; tool-schema, built-in-tools doc, guide, CHANGELOG, and ADR all match the implemented output format exactly. Commit is atomic with the message matching the issue Metadata; no merge commits. Prior review comments: none existed at review time, so no previously-raised concerns needed re-checking. Script execution remains tracked by #163 and is deliberately out of scope here. Nice work — approving. The nits are optional; address them in a follow-up if you agree with any.
@ -551,0 +615,4 @@
"tools": [{"name": "skill"}]
}
"""
And a skills context with skill "pdf-processing" having resources of every D-8 class
Member

Nit: the new D-8 matrix fixtures include the root-level file LICENSE.txt, but no scenario ever reads it. The ticket's expected behavior says root-level files remain readable in every mode; the read path is shared with reference reads so coverage isn't affected, but a one-line "Reading a root-level file from an activated skill" scenario would complete the D-8 read matrix.

Nit: the new D-8 matrix fixtures include the root-level file `LICENSE.txt`, but no scenario ever reads it. The ticket's expected behavior says root-level files remain readable in every mode; the read path is shared with reference reads so coverage isn't affected, but a one-line "Reading a root-level file from an activated skill" scenario would complete the D-8 read matrix.
@ -45,0 +50,4 @@
("asset", "assets"),
("script", "scripts"),
)
_SKILL_NON_STANDARD_CLASS = "non_standard"
Member

Nit: _SKILL_NON_STANDARD_CLASS = "non_standard" duplicates SkillResourceClass.NON_STANDARD.value from skill_schema.py. If the enum value ever changed, the gate would silently fail open (non-standard reads allowed) — the Behave gate matrix would catch the drift, but deriving the constant from the enum (tool.py otherwise consumes plain data, so a module-level SkillResourceClass.NON_STANDARD.value import or a parity assertion in tests) would be more robust.

Nit: `_SKILL_NON_STANDARD_CLASS = "non_standard"` duplicates `SkillResourceClass.NON_STANDARD.value` from `skill_schema.py`. If the enum value ever changed, the gate would silently fail open (non-standard reads allowed) — the Behave gate matrix would catch the drift, but deriving the constant from the enum (tool.py otherwise consumes plain data, so a module-level `SkillResourceClass.NON_STANDARD.value` import or a parity assertion in tests) would be more robust.
Member

Nit: while the D-9 gate is closed, this "Available resources" listing includes non-standard entries (e.g. fixtures/sample.pdf) — the same paths the activation manifest deliberately hides. ADR-2034 D-10 does say the error names the available resources, so this is spec-compliant as written; but filtering non_standard entries from this listing while the gate is closed would be consistent with D-9's hiding intent (a probing read currently reveals gated paths by name). Would require a small ADR wording amendment if you choose to filter.

Nit: while the D-9 gate is closed, this "Available resources" listing includes non-standard entries (e.g. `fixtures/sample.pdf`) — the same paths the activation manifest deliberately hides. ADR-2034 D-10 does say the error names the available resources, so this is spec-compliant as written; but filtering `non_standard` entries from this listing while the gate is closed would be consistent with D-9's hiding intent (a probing read currently reveals gated paths by name). Would require a small ADR wording amendment if you choose to filter.
CoreRasurae force-pushed feature/m1-skill-resource-manifest-and-assets from ec7a3e3d60
Some checks failed
CI / benchmark (pull_request) Has started running
CI / lint (pull_request) Successful in 1m11s
CI / build (pull_request) Successful in 1m15s
CI / typecheck (pull_request) Successful in 1m33s
CI / quality (pull_request) Successful in 2m11s
CI / security (pull_request) Successful in 2m47s
CI / integration_tests (pull_request) Successful in 4m29s
CI / unit_tests (pull_request) Successful in 6m40s
CI / coverage (pull_request) Failing after 11m11s
CI / status-check (pull_request) Has been cancelled
to ead2669a44
Some checks are pending
CI / lint (pull_request) Successful in 50s
CI / typecheck (pull_request) Successful in 1m39s
CI / quality (pull_request) Successful in 2m7s
CI / benchmark (pull_request) Has started running
CI / security (pull_request) Successful in 2m43s
CI / build (pull_request) Successful in 2m10s
CI / integration_tests (pull_request) Successful in 2m47s
CI / unit_tests (pull_request) Successful in 4m55s
CI / coverage (pull_request) Successful in 6m23s
CI / status-check (pull_request) Successful in 3s
2026-11-10 15:45:29 +00:00
Compare
Some checks are pending
CI / lint (pull_request) Successful in 50s
CI / typecheck (pull_request) Successful in 1m39s
CI / quality (pull_request) Successful in 2m7s
CI / benchmark (pull_request) Has started running
CI / security (pull_request) Successful in 2m43s
CI / build (pull_request) Successful in 2m10s
CI / integration_tests (pull_request) Successful in 2m47s
CI / unit_tests (pull_request) Successful in 4m55s
CI / coverage (pull_request) Successful in 6m23s
CI / status-check (pull_request) Successful in 3s
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feature/m1-skill-resource-manifest-and-assets:feature/m1-skill-resource-manifest-and-assets
git switch feature/m1-skill-resource-manifest-and-assets
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveractors-core!167
No description provided.