Skip to content

feat: add <tool>, <session>, <agent> directives and optional <aml> root - #8

Merged
sergio-sisternes-epam merged 11 commits into
mainfrom
sergio-sisternes-epam/skill-notation-expansion
May 24, 2026
Merged

feat: add <tool>, <session>, <agent> directives and optional <aml> root#8
sergio-sisternes-epam merged 11 commits into
mainfrom
sergio-sisternes-epam/skill-notation-expansion

Conversation

@sergio-sisternes-epam

Copy link
Copy Markdown
Owner

Summary

Expands the <skill> notation with three new directive tag types (<tool>, <session>, <agent>) and an optional <aml version="..."> document root wrapper.

Directive tags instruct agents how to execute content, complementing <skill> tags which declare what to execute. The <aml> root wrapper enables version negotiation for future spec evolution while remaining fully backward-compatible with fragment-mode parsing.

The constraint composition model was hardened through a 5-iteration, 6-agent review campaign (architect, developer, testing, PR reviewer, tech writer, code quality) that identified and fixed 12 bugs across 30 total reviews, converging to zero findings on the final iteration.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactor / chore

Checklist

  • Tests added or updated
  • Documentation updated (if applicable)
  • cargo fmt and cargo clippy pass
  • CI is green

Notes for reviewers

Directive tags

  • <tool> -- constrain which tools are available within a scope via name, allow, and deny attributes. Composition is monotonic: inner scopes can only restrict, never expand.
  • <session> -- execute content in a separate session with optional isolation and failure handling.
  • <agent> -- delegate execution to a subagent with optional model override and sync/background mode.

All three support on-failure (halt/skip/partial) except <tool> which is a pure scope directive.

Tool constraint composition

Allow-lists intersect, deny-lists union, deny always beats allow. The name attribute is shorthand for allow="<name>". Constraints propagate down the tree but errored tool nodes (e.g. mutual allow+deny) do not derive constraints for children.

<aml> root wrapper

Optional <aml version="0.1"> wrapper handled as a pre-scan before normal node parsing. Never appears in the AST -- consumed at parse time with version stored in Document.version. Only whitespace/comments allowed outside the wrapper.

Key files

  • crates/aml-core/src/ast.rs -- ToolDirective, SessionDirective, AgentDirective, DirectiveKind, Document.version
  • crates/aml-core/src/parser.rs -- directive parsing, <aml> root pre-scan, reject_nested_aml
  • crates/aml-core/src/validator.rs -- ToolConstraints composition with apply_deny/allow/name helpers
  • crates/aml-core/src/executor.rs -- directive on-failure handling
  • docs/spec/execution.md, docs/spec/security.md -- spec updates
  • docs/src/content/docs/guide/directives.mdx -- user guide

Test coverage

113 tests (84 unit + 16 integration + 13 parser), all passing. Constraint-specific tests cover: 4-level nesting, deny/allow intersection, name shorthand narrowing, errored tool propagation, deduplication, warning source attribution, and more.

Known limitations (tracked separately)

sergio-sisternes-epam and others added 11 commits May 24, 2026 01:51
Expand AML with three new top-level directive tags that instruct the
runtime about execution environment:

- <tool>: constrains which tools are available within a scope
  (name, allow, deny attributes; allow/deny mutually exclusive)
- <session>: executes enclosed content in a separate session
  (name, isolated attributes)
- <agent>: delegates execution to a subagent
  (name, model, mode attributes)

Design decisions:
- Represented as Node::Directive with typed DirectiveKind enum
- Parser generalised around TagName enum (no more hardcoded prefixes)
- Full nesting with each other and with <skill> tags
- Directives inside definition bodies are invalid
- Core executor treats directives as pass-through; runtime-specific
  behaviour is deferred to the harness
- Lifecycle gains a conceptual Authorise phase for directive constraints

Includes ADR-010, spec updates (grammar, attributes, execution, lifecycle,
security, error-model), docs site pages, usage guide, example, and 49
passing tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 16 integration tests in smoke_directives.rs covering:
- Complex document parse/validate/execute pipeline
- Nested tool narrowing (allow > allow intersection)
- Mixed deny/allow nesting across levels
- Triple-nested tool directives (3 levels deep)
- Realistic CI pipeline with 4 agents and nested tools
  using real tool names (grep, glob, view, bash, web_search, etc.)
- Tool name shorthand nesting
- Sibling tool directives within an agent
- Negative tests (allow/deny conflict, missing attrs, definition nesting)
- Self-closing directives and execution order preservation

Also adds 8 conformance test fixtures (positive, negative, execution).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hole docs

Extend AML directives based on smoke test agent feedback:

on-failure on directives:
- Add on_failure: Option<FailureMode> to SessionDirective and AgentDirective
- Parse on-failure attribute on <session> and <agent> tags
- Executor applies halt/skip/partial semantics to directive children
- Not on <tool> (constraints don't fail, they constrain)

Tool constraint composition:
- Document monotonic narrowing: allow intersect, deny union, deny beats allow
- Add validator warnings for contradictory nested tool constraints
  (e.g. inner allow requesting tools denied by ancestor)
- Severity enum (Error/Warning) on ValidationError

Bash loophole:
- Document that bash is a superuser tool undermining fine-grained constraints
- Recommend sandboxed bash or grep/view-only for read-only contexts

11 new tests (76 total, all passing). Spec, guide, and usage skill updated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix 6 bugs found by 6-agent review campaign:

Critical fixes in ToolConstraints::apply():
- Filter denied tools from effective allow-list (both branches)
- Remove newly-denied tools from inherited allowed list on deny-union
- Name shorthand now sets allowed = [name] (was no-op)

Parser enforcement:
- Reject on-failure attribute on <tool> tags (parse error)
- Filter empty strings from allow/deny comma splits

Validator warnings:
- Warn when name and allow both set on same <tool> (redundant)

8 new constraint state tests verifying effective allow-set values,
not just warnings. Updated smoke test for correct name narrowing.
84 tests total (68 unit + 16 integration), all passing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Iteration 1 campaign feedback fixes (6 agents):

Bug fixes:
- apply_allow() now uses new.denied (post-deny) not self.denied, so
  same-node deny+allow correctly excludes the denied tool
- Name shorthand no longer skipped when deny is present — guard
  changed from 'allow.is_none() && deny.is_none()' to 'allow.is_none()'

Refactor:
- Split apply() into apply_deny(), apply_allow(), apply_name() helpers
- Extracted parse_tool_list() for comma-separated values

7 new edge case tests (91 total):
- Same-node deny+allow, name+deny narrowing, self-contradictory name+deny
- Empty parent ∩ child intersection, allow∩allow state verification
- Stacked deny union across 3 levels, whitespace/comma edge cases

Doc: clarified name shorthand narrows constraint scope.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lpers

Iteration 2 campaign feedback (6 agents):

Bug fixes:
  preventing double-warning when tool is both denied and absent
- Errored <tool> (allow+deny) no longer derives constraints for children,
  eliminating spurious downstream warnings
- parse_tool_list() promoted to free function (no ToolConstraints dependency)
- apply_name() uses single name.to_string() allocation instead of 5

5 new tests (96 total):
- 4-level deep nesting, name→deny conflict at depth, parent allow+name
- Errored tool no-propagation, apply_name no-double-warning

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Iteration 3 campaign feedback (6 agents):

Bug fix:
- apply_name() now distinguishes between 'this' and 'ancestor' deny source
  in warning messages (was always saying 'ancestor' even for same-node deny)

Cleanup:
- Removed accidentally committed target/.rustc_info.json

2 new tests (98 total):
- Same-node name+deny says 'this directive'
- Ancestor deny says 'ancestor'

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Iteration 4 campaign feedback (6 agents):

Bug fix:
- parse_tool_list() now deduplicates entries, preventing duplicate warnings
  from repeated tool names in allow/deny attributes (e.g. allow="bash,bash")
- Also ensures allowed/denied lists never contain duplicates

Cleanup:
- Removed duplicate target/ entry in .gitignore

2 new tests (100 total):
- parse_tool_list deduplication
- Duplicate allow produces single warning

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
target/ is already in .gitignore — these files were accidentally committed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add an optional <aml version="0.1"> document root wrapper that declares
the AML specification version. The wrapper is handled at the parse() level
as a document-level construct — it never appears in the AST.

Behaviour:
- Fragment mode (no <aml>): unchanged, version is None
- Root mode (<aml version="...">): children become root nodes, version
  is stored in Document.version
- Only whitespace and XML comments allowed outside <aml>
- Nested <aml> tags are rejected at parse time
- version attribute is required, no other attributes allowed

AST: Document gains version: Option<String> field with backward-compatible
Document::new() (defaults to None) and Document::with_version() constructor.

13 new tests (113 total):
- Root with version, self-closing, whitespace/comments around
- Fragment mode, missing version, unknown attrs, text before/after
- Nested <aml> errors, unclosed, children preserved

Docs: Updated directives guide and AML usage guide skill.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply rustfmt formatting and fix two needless_borrow clippy warnings
in build_directive_kind (parse_on_failure calls).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam
sergio-sisternes-epam merged commit 2f4f469 into main May 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant