Add storage-fsx-windows-sla-optimizer skill

[benlec]

Description

Adds a new read-only skill, storage-fsx-windows-sla-optimizer, that performs a
structured SLA-readiness and availability review of Amazon FSx for Windows File Server
file systems and surfaces cost-optimization opportunities.

Given one or more file-system IDs (or a region to discover them in), it evaluates each
file system across seven availability dimensions — deployment type (Single-AZ vs
Multi-AZ), Active Directory health, throughput capacity (peak-aware), storage headroom,
backups, maintenance window, and CloudWatch alarm coverage — and returns a rated report
(High / Medium / Low / Indeterminate) with prioritized findings and remediation. It adds
usage-pattern trend analysis (peak-aware throughput sizing, weekday/weekend profile, and
storage-growth projection) and flags heavily over-provisioned or idle capacity as 💰
advisory cost notes that never lower the SLA rating. Single- vs multi-file-system (fleet)
reviews route automatically by count.

The skill is strictly read-only: control-plane Describe*/GetMetricData calls only, no
SMB/data-plane access, no mutating operations. It is fully covered by the
AIDevOpsAgentAccessPolicy managed policy and needs no additional IAM.

Type of change

• New skill
• New custom agent
• Update to an existing skill or agent
• Documentation or infrastructure change

Testing

Validated two complementary ways.

1. Agent Skill Eval (aws-samples/sample-agent-skill-eval) — the skill ships
evals/ (16 functional cases + 8 trigger queries + mock fixtures) and a .skilleval.yaml.

• Audit: 100/100, Grade A (0 critical, 0 warning, 0 info).
• Functional: PASSED — with-skill vs without-skill delta +22.5% across 16 cases
  (with-skill ~92–98% across runs; without-skill ~70%). Includes fixture-based cases for
  the trend engine (weekday-peaker, idle, storage-filler) and a MISCONFIGURED / Critical
  AD case (with=100% / without=0%).
• Trigger: PASSED — 100% trigger precision and 100% no-trigger precision (8/8),
  including deliberate near-miss negatives (S3 cost question; FSx for NetApp ONTAP).
• Note: the eval was run with a local coding-agent CLI as the runner (the framework's
  default claude CLI was unavailable, so a pluggable runner was used). Token/tool-call
  metadata isn't emitted by that runner, so the eval's process/efficiency sub-scores read
  as 0; the outcome/style/trigger dimensions and the with/without delta are unaffected.

2. Manual AWS DevOps Agent testing — the skill was uploaded to an AgentSpace and run
against real FSx for Windows infrastructure (a purpose-built test fleet). Confirmed live:
a Medium multi-warning case, a High case (after adding alarm coverage), the 💰
cost-optimization lens on an over-provisioned file system, and a multi-file-system fleet
review (distribution summary + comparison matrix). The Critical/MISCONFIGURED path is
covered by the fixture-based eval case above (a live MISCONFIGURED state was not forced,
as FSx does not reliably flip lifecycle from network isolation alone).

llms.txt is updated with the skill entry. The auto-generated Skills Catalog picks the
skill up from its SKILL.md frontmatter (aws-devops-agent-skills.* metadata), so no
manual README table edit is required.

License confirmation

• By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0.

[emenguy]

Overall the logic is great, I suggest to change few concepts.

[emenguy]

LGTM

[ams-thakkar]

What needs to change

The STR-011 suppression looks unnecessary, and the reason given for it isn't correct. skills/storage-fsx-windows-sla-optimizer/.skilleval.yaml:

- STR-011    # False positive: skill-eval's zero-dependency _simple_yaml_parse
             # cannot read YAML folded block scalars ("description: >") ...
             # The real description is 965 chars and is REQUIRED to be a folded
             # block by the AWS DevOps Agent upload validator (1024-char limit).

The "required to be a folded block" part is not true. Twenty of the twenty-five skills in skills/ use a plain multi-line description, including eks-upgrade-readiness at 1010 characters — fourteen under the limit — so plain style handles descriptions right up against the cap.

The parser claim doesn't survive comparison either. Three other skills already use folded or literal block descriptions and none needs this waiver: aiml-access-diagnostics at 1017 characters and storage-s3-resiliency-expertise at 981, both suppressing only STR-016, and bedrock-adoption-readiness at 652 with no .skilleval.yaml at all. Yours at 967 characters is shorter than the first of those. This is the only one of the twenty-two skills carrying a .skilleval.yaml that suppresses STR-011.

I can't run skill-eval myself, so I'm not claiming the rule passes for you — I'm saying three merged counterexamples make the stated reason the least likely explanation. Please either drop the suppression and confirm the audit is clean, or, if it genuinely fails, say why it fails here and not for the longer, also-folded aiml-access-diagnostics. Suppressing an audit rule is a durable decision and I don't want to land one on a premise that doesn't hold. Switching to plain multi-line, as the other twenty skills do, sidesteps it either way.

Nits

Non-blocking:

• The suppression comment states the description is 965 characters; I measure 967 after folding. Immaterial to the limit, but it reads like the figure wasn't re-checked. Moot if the suppression goes.

Thanks

The IAM work here is the best I've reviewed in this repo. You didn't just assert the managed policy covers the skill — you worked out that AIDevOpsAgentAccessPolicy grants fsx:Describe* but no fsx:List*, concluded fsx:ListTagsForResource is therefore unavailable, and restructured the collection to read Name and cost-allocation tags from the Tags array that describe-file-systems already returns inline. I checked that against the live published policy document and it is exactly right. The consequence is a new skill that needs no CloudFormation change at all, which is the outcome the template's per-skill gating is trying to make possible.

The rest of the boundaries hold up too. The AWSSupport-ValidateFSxWindowsADConfig runbook is only ever recommended as remediation, never executed, and README.md:83 tells the reader that running it needs ssm:StartAutomationExecution on their side. references/data-collection.md:263 marks Name tags, FailureDetails.Message, and directory fields as an untrusted data boundary, which is prompt-injection awareness I don't often see volunteered. Degrading an AccessDenied check to "Unable to verify" and capping the SLA rating at Medium rather than guessing is the right failure mode, and stopping for user confirmation instead of proceeding by default is the right default.

Verified myself: merges clean into main with no conflicts; all five required actions resolve against the live AIDevOpsAgentAccessPolicy v10 (fsx:Describe*, ds:Describe*, cloudwatch:GetMetricData, cloudwatch:Describe*), with sts:GetCallerIdentity correctly noted as needing no grant; fsx:ListTagsForResource appears nowhere except the README note explaining it isn't used; no write-shaped operation is instructed anywhere; frontmatter name matches the directory, description is 967 characters, and version: 1.0.0 matches the CHANGELOG top entry; all seven JSON files parse and .skilleval.yaml parses; extensions are limited to md/json/yaml; the README has no relative .md links and mkdocs build --strict passes with zero warnings; and llms.txt:38 carries the entry.

On evals: this ships evals/evals.json, eval_queries.json and five fixtures, but not the structure/, best-practices/, functional/ results layout. PR 72 is in PRS_PREDATING_CHECK, so the check reports WARN and exits 0; run on merit it fails. That waiver is legitimate — this PR predates the check — and I'm not asking you to backfill, since no skill on main carries the full layout yet. Flagging it so the decision is visible rather than silent.

Thanks also to @emenguy for the domain review — six inline comments, all answered, then approval. I'm treating that as the FSx correctness gate; my review is scoped to repository mechanics, the IAM surface, and the read-only boundary. Happy to approve once the STR-011 question is settled.

[benlec]

@ams-thakkar Addressed in 147ded0. I removed the STR-011 suppression entirely, converted the SKILL.md description from description: > to the repository-standard plain multiline form, and removed the stale exact character-count claim from CHANGELOG.md. Validation: PyYAML and the repository catalog parser both parse the full description at 965 characters; the public skill-eval zero-dependency parser sees a 68-character first-line description, so the exact STR-011 (len(desc) < 20) path does not fire; .skilleval.yaml now ignores only STR-016; git diff --check and the PR #72 eval-layout validator pass. The full skill-eval CLI is not installed locally, so I verified the exact published STR-011 implementation rather than claiming a complete CLI audit.

[ams-thakkar]

Resolved at 147ded052. The suppression is gone (.skilleval.yaml now ignores only STR-016), and converting the description to plain multiline preserved it exactly — both revisions parse to the same 965-character string once whitespace is folded, with every other frontmatter key byte-identical and still under the 1024 cap. I confirmed the rule no longer fires: the zero-dependency parser now reads a 68-character first line, well clear of the len < 20 threshold.

Your diagnosis was right and my framing was partly wrong — a folded scalar does collapse to > at one character, so STR-011 genuinely would have fired. My objection only held on the "required to be folded" claim and on suppression as the remedy. Fixing the input is the better outcome, and verifying the published rule implementation rather than claiming a full CLI audit is the right caveat to state.

Re-verified against current main, since #42 moved it after my first pass: merges clean with zero conflicts, mkdocs build --strict exits 0 with no warnings, the evals check reports WARN/exit 0 under the predating waiver, all seven JSON files parse, extensions stay md/json/yaml, and version: 1.0.0 matches the CHANGELOG. This revision touches three files and only the description line in SKILL.md, so the IAM surface and read-only boundaries I checked previously are unchanged.

Approving as scoped maintainer approval — repository mechanics, the IAM surface against live AIDevOpsAgentAccessPolicy v10, and the read-only boundary. FSx correctness rests on @emenguy's review. Resolving the open threads so this can merge.