cleveragents/cleveragents-core

Fork 3

fix(skills): add lowercase-only namespace/name pattern validation to SkillConfigSchema.name field #3208

Merged

freemo merged 1 commit from fix/skill-config-schema-lowercase-name-validation into master

2026-04-05 21:09:26 +00:00

freemo commented

2026-04-05 07:48:09 +00:00

Owner

Summary

Fixes a UAT-identified bug (#3029) where SkillConfigSchema and ActionConfigSchema incorrectly accepted uppercase letters in the namespace/name pattern, violating the spec's naming convention.

Problem

Both src/cleveragents/skills/schema.py and src/cleveragents/action/schema.py defined NAMESPACED_NAME_RE with a permissive pattern that allowed uppercase letters:

# Before (incorrect - allows uppercase)
NAMESPACED_NAME_RE = re.compile(
    r"^[a-zA-Z0-9][a-zA-Z0-9_-]*/[a-zA-Z0-9][a-zA-Z0-9_-]*$"
)

This contradicted the spec's JSON Schema pattern ^[a-z0-9_-]+/[a-z0-9_-]+$ which requires lowercase only.

Solution

Updated NAMESPACED_NAME_RE in both schema files to enforce lowercase-only characters:

# After (correct - lowercase only)
NAMESPACED_NAME_RE = re.compile(r"^[a-z0-9][a-z0-9_-]*/[a-z0-9][a-z0-9_-]*$")

Changes

src/cleveragents/skills/schema.py: Updated NAMESPACED_NAME_RE to reject uppercase letters in namespace and name parts (affects SkillConfigSchema.name, SkillToolRefSchema.name, and SkillIncludeSchema.name)
src/cleveragents/action/schema.py: Applied the same lowercase-only fix to ActionConfigSchema
features/skill_schema.feature: Added 5 new BDD scenarios covering:
- Uppercase namespace rejection (MyOrg/my-skill)
- Uppercase name part rejection (myorg/MySkill)
- Fully uppercase name rejection (MyOrg/MySkill)
- Uppercase tool reference name rejection
- Uppercase include name rejection
features/consolidated_action.feature: Added 3 new BDD scenarios for action schema uppercase name rejection

Testing

All new scenarios follow the existing TDD pattern — they validate that the schema raises a ValidationError with a message mentioning namespace/name when uppercase letters are present.

Verified with direct Python testing that:

MyOrg/my-skill → rejected ✓
myorg/MySkill → rejected ✓
MyOrg/MySkill → rejected ✓
local/my-skill → accepted ✓
myorg/my-action → accepted ✓

Closes #3029

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

## Summary Fixes a UAT-identified bug (#3029) where `SkillConfigSchema` and `ActionConfigSchema` incorrectly accepted uppercase letters in the `namespace/name` pattern, violating the spec's naming convention. ## Problem Both `src/cleveragents/skills/schema.py` and `src/cleveragents/action/schema.py` defined `NAMESPACED_NAME_RE` with a permissive pattern that allowed uppercase letters: ```python # Before (incorrect - allows uppercase) NAMESPACED_NAME_RE = re.compile( r"^[a-zA-Z0-9][a-zA-Z0-9_-]*/[a-zA-Z0-9][a-zA-Z0-9_-]*$" ) ``` This contradicted the spec's JSON Schema pattern `^[a-z0-9_-]+/[a-z0-9_-]+$` which requires lowercase only. ## Solution Updated `NAMESPACED_NAME_RE` in both schema files to enforce lowercase-only characters: ```python # After (correct - lowercase only) NAMESPACED_NAME_RE = re.compile(r"^[a-z0-9][a-z0-9_-]*/[a-z0-9][a-z0-9_-]*$") ``` ## Changes - **`src/cleveragents/skills/schema.py`**: Updated `NAMESPACED_NAME_RE` to reject uppercase letters in namespace and name parts (affects `SkillConfigSchema.name`, `SkillToolRefSchema.name`, and `SkillIncludeSchema.name`) - **`src/cleveragents/action/schema.py`**: Applied the same lowercase-only fix to `ActionConfigSchema` - **`features/skill_schema.feature`**: Added 5 new BDD scenarios covering: - Uppercase namespace rejection (`MyOrg/my-skill`) - Uppercase name part rejection (`myorg/MySkill`) - Fully uppercase name rejection (`MyOrg/MySkill`) - Uppercase tool reference name rejection - Uppercase include name rejection - **`features/consolidated_action.feature`**: Added 3 new BDD scenarios for action schema uppercase name rejection ## Testing All new scenarios follow the existing TDD pattern — they validate that the schema raises a `ValidationError` with a message mentioning `namespace/name` when uppercase letters are present. Verified with direct Python testing that: - `MyOrg/my-skill` → rejected ✓ - `myorg/MySkill` → rejected ✓ - `MyOrg/MySkill` → rejected ✓ - `local/my-skill` → accepted ✓ - `myorg/my-action` → accepted ✓ Closes #3029 --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker

freemo added 1 commit

2026-04-05 07:48:09 +00:00

fix(skills): add lowercase-only namespace/name pattern validation to SkillConfigSchema.name field

CI / lint (pull_request) Successful in 20s

Details

CI / typecheck (pull_request) Successful in 56s

Details

CI / security (pull_request) Successful in 1m0s

Details

CI / quality (pull_request) Successful in 41s

Details

CI / build (pull_request) Successful in 30s

Details

CI / helm (pull_request) Successful in 23s

Details

CI / unit_tests (pull_request) Successful in 7m15s

Details

CI / e2e_tests (pull_request) Successful in 16m28s

Details

CI / integration_tests (pull_request) Successful in 22m39s

Details

CI / docker (pull_request) Successful in 1m25s

Details

CI / coverage (pull_request) Successful in 11m4s

Details

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

Details

CI / benchmark-publish (pull_request) Has been skipped

Details

CI / benchmark-regression (pull_request) Successful in 57m3s

Details

7b0533b34d

Enforce lowercase-only namespace/name pattern in both SkillConfigSchema
and ActionConfigSchema by updating NAMESPACED_NAME_RE from the permissive
[a-zA-Z0-9] pattern to the strict [a-z0-9] pattern.

This fixes a UAT-identified bug where uppercase letters were incorrectly
accepted in the namespace/name fields of skill and action configurations,
violating the spec's naming convention (^[a-z0-9_-]+/[a-z0-9_-]+$).

Changes:
- src/cleveragents/skills/schema.py: Update NAMESPACED_NAME_RE to reject
  uppercase letters in namespace and name parts
- src/cleveragents/action/schema.py: Apply the same lowercase-only fix
- features/skill_schema.feature: Add BDD scenarios for uppercase namespace,
  uppercase name part, fully uppercase name, uppercase tool ref, and
  uppercase include name rejection
- features/consolidated_action.feature: Add BDD scenarios for uppercase
  namespace, uppercase name part, and fully uppercase name rejection

ISSUES CLOSED: #3029

freemo referenced this pull request

2026-04-05 07:51:57 +00:00

UAT: SkillConfigSchema does not validate that name field follows namespace/name pattern with lowercase-only characters — uppercase letters are incorrectly accepted #3029

freemo commented

2026-04-05 08:28:26 +00:00

Author

Owner

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3208-1775372600]

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3208-1775372600] --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

freemo reviewed

2026-04-05 09:18:23 +00:00

freemo left a comment

✅ Independent Code Review — Approved

Summary

This PR correctly fixes a UAT-identified bug (#3029) where SkillConfigSchema and ActionConfigSchema accepted uppercase letters in namespace/name fields, violating the spec's lowercase-only naming convention.

What Was Reviewed

Regex change in src/cleveragents/skills/schema.py: NAMESPACED_NAME_RE updated from [a-zA-Z0-9] to [a-z0-9] — correctly enforces lowercase. Matches the spec's JSON Schema pattern at docs/specification.md line ~30886: ^[a-z0-9_-]+/[a-z0-9_-]+$.
Regex change in src/cleveragents/action/schema.py: Same fix applied consistently.
5 new BDD scenarios in features/skill_schema.feature: Cover uppercase namespace, uppercase name part, fully uppercase, uppercase tool ref, and uppercase include — thorough edge case coverage.
3 new BDD scenarios in features/consolidated_action.feature: Cover the same uppercase rejection patterns for actions.
Commit message: Follows Conventional Changelog format with proper ISSUES CLOSED: #3029 footer.

Verification

All required CI checks passing (lint ✅, typecheck ✅, security ✅, quality ✅, unit_tests ✅, integration_tests ✅, e2e_tests ✅, coverage ✅)
Change is minimal, focused, and correct
Tests are meaningful BDD scenarios, not coverage padding
No # type: ignore suppressions
No security concerns

Minor Observations (non-blocking)

Schema YAML files not updated: docs/schema/skill.schema.yaml and docs/schema/action.schema.yaml still use the old [a-zA-Z0-9_-]+ pattern. These should be updated in a follow-up to stay consistent with the Python implementation. This is pre-existing inconsistency, not introduced by this PR.
First-char restriction: The implementation pattern [a-z0-9][a-z0-9_-]* is slightly more restrictive than the spec's [a-z0-9_-]+ (disallows _ or - as first character). This is pre-existing behavior preserved by this PR — not a concern here.

Decision

APPROVED — Clean, well-tested bug fix that aligns the implementation with the spec's lowercase naming convention. Proceeding with merge.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## ✅ Independent Code Review — Approved ### Summary This PR correctly fixes a UAT-identified bug (#3029) where `SkillConfigSchema` and `ActionConfigSchema` accepted uppercase letters in `namespace/name` fields, violating the spec's lowercase-only naming convention. ### What Was Reviewed - **Regex change in `src/cleveragents/skills/schema.py`**: `NAMESPACED_NAME_RE` updated from `[a-zA-Z0-9]` to `[a-z0-9]` — correctly enforces lowercase. Matches the spec's JSON Schema pattern at `docs/specification.md` line ~30886: `^[a-z0-9_-]+/[a-z0-9_-]+$`. - **Regex change in `src/cleveragents/action/schema.py`**: Same fix applied consistently. - **5 new BDD scenarios in `features/skill_schema.feature`**: Cover uppercase namespace, uppercase name part, fully uppercase, uppercase tool ref, and uppercase include — thorough edge case coverage. - **3 new BDD scenarios in `features/consolidated_action.feature`**: Cover the same uppercase rejection patterns for actions. - **Commit message**: Follows Conventional Changelog format with proper `ISSUES CLOSED: #3029` footer. ### Verification - All required CI checks passing (lint ✅, typecheck ✅, security ✅, quality ✅, unit_tests ✅, integration_tests ✅, e2e_tests ✅, coverage ✅) - Change is minimal, focused, and correct - Tests are meaningful BDD scenarios, not coverage padding - No `# type: ignore` suppressions - No security concerns ### Minor Observations (non-blocking) 1. **Schema YAML files not updated**: `docs/schema/skill.schema.yaml` and `docs/schema/action.schema.yaml` still use the old `[a-zA-Z0-9_-]+` pattern. These should be updated in a follow-up to stay consistent with the Python implementation. This is pre-existing inconsistency, not introduced by this PR. 2. **First-char restriction**: The implementation pattern `[a-z0-9][a-z0-9_-]*` is slightly more restrictive than the spec's `[a-z0-9_-]+` (disallows `_` or `-` as first character). This is pre-existing behavior preserved by this PR — not a concern here. ### Decision **APPROVED** — Clean, well-tested bug fix that aligns the implementation with the spec's lowercase naming convention. Proceeding with merge. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer

freemo scheduled this pull request to auto merge when all checks succeed

2026-04-05 09:20:38 +00:00

freemo referenced this pull request

2026-04-05 10:05:15 +00:00

UAT: SkillConfigSchema does not validate that name field follows namespace/name pattern with lowercase-only characters — uppercase letters are incorrectly accepted #3029

freemo referenced this pull request

2026-04-05 10:18:18 +00:00

UAT: SkillConfigSchema does not validate that name field follows namespace/name pattern with lowercase-only characters — uppercase letters are incorrectly accepted #3029

freemo reviewed

2026-04-05 16:34:01 +00:00

freemo left a comment

Independent Code Review — APPROVED ✅

Reviewed PR #3208 with focus on resource-management, memory-leaks, and cleanup-patterns, plus all standard review criteria.

Changes Reviewed

File	Change
`src/cleveragents/skills/schema.py`	`NAMESPACED_NAME_RE` regex updated from `[a-zA-Z0-9]` → `[a-z0-9]` (lowercase-only enforcement)
`src/cleveragents/action/schema.py`	Same regex fix applied consistently
`features/skill_schema.feature`	+5 BDD scenarios for uppercase rejection (namespace, name, both, tool ref, include)
`features/consolidated_action.feature`	+3 BDD scenarios for uppercase rejection (namespace, name, both)

Standard Criteria

✅ Specification Compliance: The fix correctly aligns the Python regex with the spec's JSON Schema pattern ^[a-z0-9_-]+/[a-z0-9_-]+$ (lowercase-only). The implementation pattern ^[a-z0-9][a-z0-9_-]*/[a-z0-9][a-z0-9_-]*$ is slightly more restrictive (disallows _/- as first char), but this is pre-existing behavior preserved by this PR.

✅ Commit Message: Follows Conventional Changelog format: fix(skills): add lowercase-only namespace/name pattern validation... with proper ISSUES CLOSED: #3029 footer.

✅ PR Metadata: Has closing keyword (Closes #3029), Type/Bug label present.

✅ Code Quality: No # type: ignore suppressions. Imports at top of file. Files well under 500 lines. Clean, minimal, focused change.

✅ Test Quality: 8 new BDD scenarios are meaningful behavior verification covering multiple edge cases (uppercase in namespace only, name only, both parts, tool refs, includes). Tests verify actionable error messages mentioning "namespace/name". Not coverage padding.

✅ Correctness: The regex change is straightforward and correct. Both schema files are updated consistently. No logic errors or boundary condition issues.

Deep Dive: Resource Management, Memory Leaks, Cleanup Patterns

Given special attention to resource management concerns:

Compiled regex (NAMESPACED_NAME_RE): Module-level re.compile() — compiled once at import time, reused for all validations. This is the correct pattern. No memory leak. ✅
Pydantic @field_validator decorators: Stateless @classmethod validators that don't hold resources. No cleanup needed. ✅
YAML factory methods (from_yaml, from_yaml_file): Pre-existing patterns not modified by this PR. yaml.safe_load() doesn't hold resources. pathlib.Path.read_text() properly manages file handles internally. ✅
No new resource allocation: This PR introduces no new file handles, connections, sockets, threads, or other resources requiring cleanup. The change is purely a regex pattern modification. ✅
No resource lifecycle changes: The NAMESPACED_NAME_RE constant remains a module-level singleton — no risk of repeated compilation or GC pressure. ✅

Conclusion: No resource management, memory leak, or cleanup concerns in this PR.

Process Note (Non-blocking)

⚠️ Missing milestone: The PR has no milestone assigned. The linked issue #3029 is in milestone v3.6.0. Per CONTRIBUTING.md, every PR must be assigned to the same milestone as its linked issue. This should be set before merge.

Minor Observations (Non-blocking)

The docs/schema/skill.schema.yaml and docs/schema/action.schema.yaml files still use the old [a-zA-Z0-9_-]+ pattern. A follow-up issue should be created to update these schema definition files for consistency. This is pre-existing inconsistency, not introduced by this PR.

Decision: APPROVED ✅

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## Independent Code Review — APPROVED ✅ Reviewed PR #3208 with focus on **resource-management**, **memory-leaks**, and **cleanup-patterns**, plus all standard review criteria. ### Changes Reviewed | File | Change | |------|--------| | `src/cleveragents/skills/schema.py` | `NAMESPACED_NAME_RE` regex updated from `[a-zA-Z0-9]` → `[a-z0-9]` (lowercase-only enforcement) | | `src/cleveragents/action/schema.py` | Same regex fix applied consistently | | `features/skill_schema.feature` | +5 BDD scenarios for uppercase rejection (namespace, name, both, tool ref, include) | | `features/consolidated_action.feature` | +3 BDD scenarios for uppercase rejection (namespace, name, both) | ### Standard Criteria ✅ **Specification Compliance**: The fix correctly aligns the Python regex with the spec's JSON Schema pattern `^[a-z0-9_-]+/[a-z0-9_-]+$` (lowercase-only). The implementation pattern `^[a-z0-9][a-z0-9_-]*/[a-z0-9][a-z0-9_-]*$` is slightly more restrictive (disallows `_`/`-` as first char), but this is pre-existing behavior preserved by this PR. ✅ **Commit Message**: Follows Conventional Changelog format: `fix(skills): add lowercase-only namespace/name pattern validation...` with proper `ISSUES CLOSED: #3029` footer. ✅ **PR Metadata**: Has closing keyword (`Closes #3029`), `Type/Bug` label present. ✅ **Code Quality**: No `# type: ignore` suppressions. Imports at top of file. Files well under 500 lines. Clean, minimal, focused change. ✅ **Test Quality**: 8 new BDD scenarios are meaningful behavior verification covering multiple edge cases (uppercase in namespace only, name only, both parts, tool refs, includes). Tests verify actionable error messages mentioning "namespace/name". Not coverage padding. ✅ **Correctness**: The regex change is straightforward and correct. Both schema files are updated consistently. No logic errors or boundary condition issues. ### Deep Dive: Resource Management, Memory Leaks, Cleanup Patterns Given special attention to resource management concerns: 1. **Compiled regex (`NAMESPACED_NAME_RE`)**: Module-level `re.compile()` — compiled once at import time, reused for all validations. This is the correct pattern. No memory leak. ✅ 2. **Pydantic `@field_validator` decorators**: Stateless `@classmethod` validators that don't hold resources. No cleanup needed. ✅ 3. **YAML factory methods (`from_yaml`, `from_yaml_file`)**: Pre-existing patterns not modified by this PR. `yaml.safe_load()` doesn't hold resources. `pathlib.Path.read_text()` properly manages file handles internally. ✅ 4. **No new resource allocation**: This PR introduces no new file handles, connections, sockets, threads, or other resources requiring cleanup. The change is purely a regex pattern modification. ✅ 5. **No resource lifecycle changes**: The `NAMESPACED_NAME_RE` constant remains a module-level singleton — no risk of repeated compilation or GC pressure. ✅ **Conclusion**: No resource management, memory leak, or cleanup concerns in this PR. ### Process Note (Non-blocking) ⚠️ **Missing milestone**: The PR has no milestone assigned. The linked issue #3029 is in milestone `v3.6.0`. Per CONTRIBUTING.md, every PR must be assigned to the same milestone as its linked issue. This should be set before merge. ### Minor Observations (Non-blocking) 1. The `docs/schema/skill.schema.yaml` and `docs/schema/action.schema.yaml` files still use the old `[a-zA-Z0-9_-]+` pattern. A follow-up issue should be created to update these schema definition files for consistency. This is pre-existing inconsistency, not introduced by this PR. **Decision: APPROVED** ✅ --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer

freemo referenced this pull request

2026-04-05 21:00:52 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo merged commit 539f500abe into master

2026-04-05 21:09:25 +00:00