Files
hurui200320 2005b8ef82
CI / push-validation (push) Successful in 18s
CI / build (push) Successful in 19s
CI / helm (push) Successful in 24s
CI / lint (push) Successful in 29s
CI / security (push) Successful in 1m11s
CI / e2e_tests (push) Successful in 2m56s
CI / quality (push) Successful in 3m40s
CI / typecheck (push) Successful in 3m59s
CI / integration_tests (push) Successful in 4m3s
CI / unit_tests (push) Successful in 4m55s
CI / docker (push) Successful in 10s
CI / coverage (push) Successful in 10m44s
CI / status-check (push) Successful in 1s
CI / benchmark-regression (push) Has been skipped
CI / benchmark-publish (push) Successful in 1h13m28s
feat(tests): replace all @skip tags with proper @tdd_expected_fail tags or remove them across the entire codebase (#7221)
## Summary

Replaces all 234 bare `@skip` occurrences across 82 Behave feature files with the correct TDD issue-capture tagging system described in CONTRIBUTING.md § Bug Fix Workflow.

Previously, the noxfile ran Behave with `--tags=not @skip`, silently excluding all `@skip`-tagged scenarios from every CI run. These tests never ran, never inverted results via the `@tdd_expected_fail` mechanism, and never contributed to coverage — defeating the purpose of TDD issue-capture testing. Every `@skip` occurrence had a commented-out hint line immediately above it showing the intended proper tags (e.g., `# @tdd_issue @tdd_issue_4272 @tdd_expected_fail @skip`), confirming they were all intended for conversion.

## Changes

### Mechanical conversion (234 replacements across 82 files)
- Extracted the proper TDD tags from the comment hint above each `@skip` line, removed `@skip` from the tag set, and replaced the `@skip` line with those tags.
- Removed the now-redundant comment hint lines alongside each replacement.

### Bug-fixed scenarios — `@tdd_expected_fail` removed (84 scenarios)
- After conversion, ran `nox -s unit_tests` to identify which newly-enabled `@tdd_expected_fail` scenarios now **pass** (their referenced bugs have already been fixed). Removed `@tdd_expected_fail` from those 84 scenarios and their corresponding feature-level tags, leaving only the permanent `@tdd_issue @tdd_issue_<N>` regression-guard tags.
- Affected features include: `tdd_tool_runner_env_precedence`, `tdd_automation_profile_session_leak`, `tls_certificate_check`, `project_create_persist`, `resource_type_bootstrap_*`, and 18 others.

### Noxfile cleanup
- Removed all four `--tags=not @skip` arguments from `noxfile.py` (unit_tests and coverage sessions). With zero `@skip` tags remaining in the codebase, this filter was dead code and its presence would mislead future maintainers into thinking `@skip` is still a supported escape mechanism.

### Regression guard files
- Split the regression guards into two focused files:
  - `tdd_regression_guards_exec_env.feature` for bug #4281 (exec-env precedence)
  - `tdd_regression_guards_session_list.feature` for bug #4271 (session list summary)
- Each file carries only its own `@tdd_issue` tags at the feature level, avoiding cross-contamination via Behave tag inheritance. The `Background` step (`session-list-summary mock`) only appears in the session-list file where it is actually needed.

### Duplicate tag cleanup
- Removed duplicate `@tdd_issue @tdd_issue_4287` tag lines in `tdd_skill_add_regression.feature` (lines 20 and 29).

### Inline comment for retained `@tdd_expected_fail`
- Added inline comment in `ci_workflow_validation.feature:134` explaining why this specific #4227 scenario retains `@tdd_expected_fail` despite #4227 being closed (CI YAML does not encode threshold as a machine-readable value).

### Known edge cases — `@tdd_expected_fail` retained (closed issues, fix on master, scenarios still fail)
The following issues are **closed** and their fixes **are on master**, but the specific test assertions still fail because the fixes address other aspects of the bugs. The `@tdd_expected_fail` tags are functionally correct and must remain until the specific scenario assertions pass:
- `tdd_exec_env_resolution_precedence.feature` — bug #1080 (closed 2026-03-31). The precedence-level-2-vs-4 scenario still fails.
- `session_list_summary_dedup.feature` — bug #3046 (closed 2026-04-05). The dedup-consistency scenarios still fail.
- `actor_add_update_enforcement.feature` — bug #2609 (closed 2026-04-05). The enforcement scenarios still fail.
- `ci_workflow_validation.feature:134` — #4227 (closed 2026-04-08). The CI YAML threshold assertion still fails.

## Verification

- `grep -r "@skip" features/ --include="*.feature"` → **zero results** ✓
- `grep -n "tags=not @skip" noxfile.py` → **zero results** ✓
- `nox -s unit_tests` → **629 features passed, 0 failed** ✓ (up from ~545 before this PR)
- CI all green (coverage ≥ 97%) ✓
- Integration tests (Robot Framework) do not use `@skip` — confirmed no action needed ✓
- E2E tests (Robot Framework) do not use `@skip` — confirmed no action needed ✓
- `CHANGELOG.md` updated with entry for this change ✓
- `CONTRIBUTORS.md` — Rui Hu already listed ✓

## Issues Addressed

Closes #7025

Co-authored-by: CleverThis <hal9000@cleverthis.com>
Reviewed-on: #7221
Reviewed-by: HAL 9000 <HAL9000@cleverthis.com>
Reviewed-by: HAL9001 <hal9001@cleverthis.com>
Co-authored-by: Rui Hu <rui.hu@cleverthis.com>
Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
2026-04-13 04:56:01 +00:00

417 lines
22 KiB
Gherkin

Feature: Skill CLI commands
As a developer
I want to manage skills via CLI commands
So that I can register, inspect, and manage reusable tool collections
Background:
Given a skill CLI test runner
And the skill service is reset
# ───────────────────────────────────────────────────────
# skill add — registration
# ───────────────────────────────────────────────────────
Scenario: Add skill from valid config file
Given a valid skill config YAML file at a temp path
When I run skill CLI add with --config pointing to the YAML file
Then the skill CLI add should succeed
And the skill CLI output should contain "Skill Registered"
And the skill CLI output should contain "local/file-reader"
And the skill CLI output should contain " OK"
Scenario: Add skill with all sections populated
Given a full skill config YAML file at a temp path
When I run skill CLI add with --config pointing to the YAML file
Then the skill CLI add should succeed
And the skill CLI output should contain "Skill Registered"
And the skill CLI output should contain "Tool Sources"
Scenario: Add skill with duplicate name fails without --update
Given a valid skill config YAML file at a temp path
And the skill "local/file-reader" is already registered
When I run skill CLI add with --config pointing to the YAML file
Then the skill CLI command should abort
And the skill CLI output should contain "already registered"
And the skill CLI output should contain "--update"
Scenario: Add skill with --update overwrites existing
Given a valid skill config YAML file at a temp path
And the skill "local/file-reader" is already registered
When I run skill CLI add with --config and --update pointing to the YAML file
Then the skill CLI add should succeed
And the skill CLI output should contain "Skill Updated"
And the skill CLI output should contain "Changes"
Scenario: Add skill with missing config file fails
When I run skill CLI add with --config pointing to a missing file
Then the skill CLI command should abort
Scenario: Add skill with invalid YAML fails
Given an invalid skill YAML file at a temp path
When I run skill CLI add with --config pointing to the YAML file
Then the skill CLI command should abort
Scenario: Add skill with --format json produces valid JSON
Given a valid skill config YAML file at a temp path
When I run skill CLI add with --config and --format json
Then the skill CLI add should succeed
And the skill CLI output should be valid JSON
Scenario: Add skill with --format yaml produces valid YAML
Given a valid skill config YAML file at a temp path
When I run skill CLI add with --config and --format yaml
Then the skill CLI add should succeed
And the skill CLI output should be valid YAML
# ───────────────────────────────────────────────────────
# skill show — details
# ───────────────────────────────────────────────────────
Scenario: Show skill displays all panels
Given the skill "local/file-reader" is registered with tools
When I run skill CLI show "local/file-reader"
Then the skill CLI show should succeed
And the skill CLI output should contain "Skill Details"
And the skill CLI output should contain "local/file-reader"
And the skill CLI output should contain "Capability Summary"
And the skill CLI output should contain " OK"
Scenario: Show skill not found
When I run skill CLI show "local/nonexistent"
Then the skill CLI command should abort
And the skill CLI output should contain "not found"
Scenario: Show skill with --format json
Given the skill "local/file-reader" is registered with tools
When I run skill CLI show "local/file-reader" with --format json
Then the skill CLI show should succeed
And the skill CLI output should be valid JSON
# ───────────────────────────────────────────────────────
# skill tools — flattened tool list
# ───────────────────────────────────────────────────────
Scenario: Tools command shows resolved tool list
Given the skill "local/file-reader" is registered with tools
When I run skill CLI tools "local/file-reader"
Then the skill CLI tools should succeed
And the skill CLI output should contain "Tools for local/file-reader"
And the skill CLI output should contain "Summary"
And the skill CLI output should contain " OK"
Scenario: Tools command with includes shows source skill
Given a composed skill "local/composed" including "local/file-reader" is registered
When I run skill CLI tools "local/composed"
Then the skill CLI tools should succeed
And the skill CLI output should contain "local/file-re"
Scenario: Tools command for nonexistent skill fails
When I run skill CLI tools "local/nonexistent"
Then the skill CLI command should abort
And the skill CLI output should contain "not found"
Scenario: Tools with --format json
Given the skill "local/file-reader" is registered with tools
When I run skill CLI tools "local/file-reader" with --format json
Then the skill CLI tools should succeed
And the skill CLI output should be valid JSON
# ───────────────────────────────────────────────────────
# skill list — listing and filtering
# ───────────────────────────────────────────────────────
Scenario: List shows all registered skills in table
Given the skill "local/file-reader" is registered with tools
And the skill "local/git-ops" is registered with tools
When I run skill CLI list
Then the skill CLI list should succeed
And the skill CLI output should contain "local/file-reader"
And the skill CLI output should contain "local/git-ops"
And the skill CLI output should contain "Summary"
Scenario: List with namespace filter
Given the skill "local/file-reader" is registered with tools
And the skill "devops/deploy-tools" is registered with tools
When I run skill CLI list with --namespace "local"
Then the skill CLI list should succeed
And the skill CLI output should contain "local/file-reader"
And the skill CLI output should not contain "devops/deploy-tools"
Scenario: List with --source mcp filter
Given a skill "local/mcp-backed" with MCP servers is registered
And the skill "local/file-reader" is registered with tools
When I run skill CLI list with --source "mcp"
Then the skill CLI list should succeed
And the skill CLI output should contain "local/mcp-backed"
And the skill CLI output should not contain "local/file-reader"
Scenario: List with no skills shows helpful message
When I run skill CLI list
Then the skill CLI output should contain "No skills found"
Scenario: List with --format json
Given the skill "local/file-reader" is registered with tools
When I run skill CLI list with --format json
Then the skill CLI list should succeed
And the skill CLI output should be valid JSON
# ───────────────────────────────────────────────────────
# skill remove — removal
# ───────────────────────────────────────────────────────
Scenario: Remove skill with --yes skips confirmation
Given the skill "local/file-reader" is registered with tools
When I run skill CLI remove "local/file-reader" with --yes
Then the skill CLI remove should succeed
And the skill CLI output should contain "Skill Removed"
And the skill CLI output should contain " OK"
Scenario: Remove nonexistent skill fails
When I run skill CLI remove "local/nonexistent" with --yes
Then the skill CLI command should abort
And the skill CLI output should contain "not found"
Scenario: Remove skill shows dependency check
Given a composed skill "local/composed" including "local/file-reader" is registered
When I run skill CLI remove "local/file-reader" with --yes
Then the skill CLI remove should succeed
And the skill CLI output should contain "Dependency Check"
And the skill CLI output should contain "local/composed"
Scenario: Remove skill with --format json
Given the skill "local/file-reader" is registered with tools
When I run skill CLI remove "local/file-reader" with --yes and --format json
Then the skill CLI remove should succeed
And the skill CLI output should be valid JSON
# ───────────────────────────────────────────────────────
# Include cycle detection
# ───────────────────────────────────────────────────────
Scenario: Tools command detects circular includes
Given a skill "local/cycle-a" including "local/cycle-b" is registered
And a skill "local/cycle-b" including "local/cycle-a" is registered
When I run skill CLI tools "local/cycle-a"
Then the skill CLI command should abort
And the skill CLI output should contain "Cycle detected"
# ───────────────────────────────────────────────────────
# Coverage — show with rich panels
# ───────────────────────────────────────────────────────
Scenario: Show skill with includes shows Includes panel
Given a composed skill "local/composed" including "local/file-reader" is registered
When I run skill CLI show "local/composed"
Then the skill CLI show should succeed
And the skill CLI output should contain "Includes"
And the skill CLI output should contain "local/file-reader"
Scenario: Show skill with MCP servers shows MCP panel and tools table
Given a skill "local/mcp-detail" with MCP servers and tools is registered
When I run skill CLI show "local/mcp-detail"
Then the skill CLI show should succeed
And the skill CLI output should contain "MCP Servers"
And the skill CLI output should contain "Direct Tools"
Scenario: Show skill with agent_skills and inline tools
Given a skill "local/mixed-sources" with agent_skills and inline tools is registered
When I run skill CLI show "local/mixed-sources"
Then the skill CLI show should succeed
And the skill CLI output should contain "Direct Tools"
Scenario: Show skill with dependents shows Referenced By
Given a composed skill "local/parent" including "local/base-tools" is registered
When I run skill CLI show "local/base-tools"
Then the skill CLI show should succeed
And the skill CLI output should contain "Referenced By"
And the skill CLI output should contain "local/parent"
# ───────────────────────────────────────────────────────
# Coverage — tools with varied source types
# ───────────────────────────────────────────────────────
Scenario: Tools command with MCP and inline entries
Given a skill "local/mixed-tools" with MCP servers and inline tools is registered
When I run skill CLI tools "local/mixed-tools"
Then the skill CLI tools should succeed
And the skill CLI output should contain "mcp:"
And the skill CLI output should contain "custom"
# ───────────────────────────────────────────────────────
# Coverage — list with non-local namespace
# ───────────────────────────────────────────────────────
Scenario: List includes non-local namespace in summary
Given the skill "local/file-reader" is registered with tools
And the skill "devops/deploy-tools" is registered with tools
When I run skill CLI list
Then the skill CLI list should succeed
And the skill CLI output should contain "Server"
And the skill CLI output should contain "devops/deploy-tools"
# ───────────────────────────────────────────────────────
# Coverage — remove without --yes
# ───────────────────────────────────────────────────────
Scenario: Remove skill without --yes prompts and aborts on deny
Given the skill "local/file-reader" is registered with tools
When I run skill CLI remove "local/file-reader" without --yes and deny
Then the skill CLI command should abort
# ───────────────────────────────────────────────────────
# Coverage — update with dependent skills
# ───────────────────────────────────────────────────────
Scenario: Update skill with dependents shows Affected Actors panel
Given a full skill config YAML file at a temp path
And a composed skill "local/uses-full" including "local/full-skill" is registered
When I run skill CLI add with --config and --update pointing to the YAML file
Then the skill CLI add should succeed
And the skill CLI output should contain "Affected Actors"
And the skill CLI output should contain "local/uses-full"
# ───────────────────────────────────────────────────────
# Coverage — skill_service edge cases
# ───────────────────────────────────────────────────────
Scenario: Service skill_count returns correct count
Given the skill "local/file-reader" is registered with tools
And the skill "local/git-ops" is registered with tools
Then the skill service should report skill count 2
Scenario: Service add_skill without config_path
Given a skill is registered without a config path
Then the config path should be empty for the skill
Scenario: Service list_skills by source builtin
Given a skill "local/builtin-src" with only tool_refs is registered
When I list skills with source "builtin"
Then the skill list should contain "local/builtin-src"
Scenario: Service list_skills by source agent_skill
Given a skill "local/agent-src" with agent_skills is registered
When I list skills with source "agent_skill"
Then the skill list should contain "local/agent-src"
Scenario: Service remove_skill with empty name raises error
Then removing a skill with empty name should raise ValueError
Scenario: Service remove_skill with nonexistent name raises error
Then removing skill "local/phantom" should raise KeyError
Scenario: Service get_skill with empty name raises error
Then getting a skill with empty name should raise ValueError
Scenario: Add skill with no-description config fails validation
Given a skill config YAML with no description at a temp path
When I run skill CLI add with --config pointing to the YAML file
Then the skill CLI command should abort
# ───────────────────────────────────────────────────────
# skill refresh — recompute flattening and sync
# ───────────────────────────────────────────────────────
Scenario: Refresh single skill recomputes tool flattening
Given the skill "local/file-reader" is registered with tools
When I run skill CLI refresh "local/file-reader"
Then the skill CLI refresh should succeed
And the skill CLI output should contain "Skill Refreshed"
And the skill CLI output should contain "local/file-reader"
And the skill CLI output should contain "Total Tools"
And the skill CLI output should contain "Read-Only"
And the skill CLI output should contain " OK"
@tdd_issue @tdd_issue_4272 @tdd_expected_fail
Scenario: Refresh single skill with --format json
Given the skill "local/file-reader" is registered with tools
When I run skill CLI refresh "local/file-reader" with --format json
Then the skill CLI refresh should succeed
And the skill CLI output should be valid JSON
And the skill CLI JSON output should have field "refreshed"
And the skill CLI JSON output should have field "skills"
Scenario: Refresh all skills with --all
Given the skill "local/file-reader" is registered with tools
And the skill "local/git-ops" is registered with tools
When I run skill CLI refresh with --all
Then the skill CLI refresh should succeed
And the skill CLI output should contain "Skills Refreshed"
And the skill CLI output should contain "local/file-reader"
And the skill CLI output should contain "local/git-ops"
Scenario: Refresh with both name and --all fails
Given the skill "local/file-reader" is registered with tools
When I run skill CLI refresh "local/file-reader" with --all
Then the skill CLI command should abort
And the skill CLI output should contain "Cannot specify both"
Scenario: Refresh without name or --all fails
When I run skill CLI refresh without arguments
Then the skill CLI command should abort
And the skill CLI output should contain "Must specify either"
Scenario: Refresh nonexistent skill fails
When I run skill CLI refresh "local/nonexistent"
Then the skill CLI command should abort
And the skill CLI output should contain "not found"
Scenario: Refresh skill with MCP servers shows sync status
Given a skill "local/mcp-backed" with MCP servers is registered
When I run skill CLI refresh "local/mcp-backed"
Then the skill CLI refresh should succeed
And the skill CLI output should contain "MCP Servers"
Scenario: Refresh all with empty registry shows helpful message
When I run skill CLI refresh with --all
Then the skill CLI output should contain "No skills registered"
Scenario: Refresh all shows table with multiple skills
Given the skill "local/file-reader" is registered with tools
And the skill "local/git-ops" is registered with tools
When I run skill CLI refresh with --all
Then the skill CLI refresh should succeed
And the skill CLI output should contain "Skills Refreshed"
# ───────────────────────────────────────────────────────
# Enhanced outputs — capability summary in JSON/YAML
# ───────────────────────────────────────────────────────
@tdd_issue @tdd_issue_4272 @tdd_expected_fail
Scenario: List with --format json includes capability summary
Given the skill "local/file-reader" is registered with tools
When I run skill CLI list with --format json
Then the skill CLI list should succeed
And the skill CLI output should be valid JSON
And the skill CLI JSON output should have field "capability_summary" in first skill
@tdd_issue @tdd_issue_4272 @tdd_expected_fail
Scenario: Show with --format json includes capability summary
Given the skill "local/file-reader" is registered with tools
When I run skill CLI show "local/file-reader" with --format json
Then the skill CLI show should succeed
And the skill CLI output should be valid JSON
And the skill CLI JSON output should have field "capability_summary"
And the skill CLI JSON output should have field "tool_count"
@tdd_issue @tdd_issue_4272 @tdd_expected_fail
Scenario: Tools with --format json includes capability summary
Given the skill "local/file-reader" is registered with tools
When I run skill CLI tools "local/file-reader" with --format json
Then the skill CLI tools should succeed
And the skill CLI output should be valid JSON
And the skill CLI JSON output should have field "capability_summary"
And the skill CLI JSON output should have field "skill_name"
And the skill CLI JSON output should have field "tools"
Scenario: List rich output shows Description column
Given the skill "local/file-reader" is registered with tools
When I run skill CLI list
Then the skill CLI list should succeed
And the skill CLI output should contain "Description"
Scenario: Show with includes displays tool count from includes
Given a composed skill "local/composed" including "local/file-reader" is registered
When I run skill CLI show "local/composed"
Then the skill CLI show should succeed
And the skill CLI output should contain "tools"