[midakin]

Description

Documentation-only update to the aws-routing skill clarifying Transit Gateway behavior with Direct Connect Gateways. It adds the DXGW-one-path-per-prefix mechanic and the same-ASN requirement for cross-DXGW ECMP to references/tgw-routing-patterns.md, so the agent stops recommending active/active ECMP across VIFs on a single DXGW behind a TGW.

Changes

• New subsection - DXGW advertises only ONE best path per prefix to TGW: a single DXGW aggregates its VIFs and propagates one winner per prefix to the TGW, so redundant VIFs on one DXGW are active/passive at the TGW; multiple paths per prefix at the TGW require multiple DXGW attachments. (Same mechanic as DXGW to CNE on Cloud WAN.)
• Clarified ECMP across DXGWs: requires the same ASN (identical AS-path, not just equal length) because TGW does not support AS-path relax. Explicitly distinguished from DX-side ECMP across VIFs, where per the public DX routing and BGP doc the AS_PATH ASNs need not match.
• Removed the line Multiple DX-GW attachments (within the same DX-GW): TGW will ECMP across matching prefixes. This reverses the previous guidance, so it needs SME review (see below).
• Marked the DXGW-one-path and same-ASN behaviors as field knowledge (validate in-account with search-transit-gateway-routes and list-virtual-interface-routes), consistent with how the doc already flags the 5,000-route advertisement-selection behavior.
• Bumped SKILL.md to 1.2.5 (main is at 1.2.4 after Set agent types from aws-devops-agent-skills.agent-types #152) with a matching [1.2.5] CHANGELOG entry.

Status of review feedback

• Rebased onto current main; no conflicts in SKILL.md / CHANGELOG.md.
• Skill evaluation tool results committed under evals/ (see Testing). SKILL.md updated to address the best-practices findings (description phrasing, linked references with load conditions, checklist + self-check); no technical guidance changed.
• Run locally against the head commit: validate_skill_evals.py PASS, scan_aws_identifiers.py 0 findings, validate_skills.py 0 errors.
• Pending: SME sign-off on the one-path-per-prefix behavior.

Type of change

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

Required before review

• Repository layout is correct — skills in /skills, MCP servers in /mcp, custom agents in /custom-agents; skills and MCP servers are never mixed in one directory.
• A domain SME has reviewed and signed off. A named subject-matter expert for this domain — someone other than the author — has reviewed the tool for technical correctness and approved it in this PR. Required for any change to what the tool tells the agent; an edit that changes no technical claim can be marked n/a with the reason, as CONTRIBUTING.md describes.
• Evaluation evidence is included (see Testing).
• No internal-only or proprietary content — no internal hostnames, account IDs, customer names, ticket numbers, internal URLs, or secrets in the tool, its examples, or its tests.
• README is complete — prerequisites, setup/deployment, and at least one sample prompt or usage example.
• CHANGELOG updated and the version bumped.
• Named owner for ongoing maintenance. (@midakin)

Testing

• structure checks pass.
• best-practices run across multiple iterations, with consistent results.
• functional run with and without the change, so the difference in the agent's behavior is visible.
• evals/evals.json includes positive cases (should trigger) and at least one negative case (should not trigger on unrelated input).

Results from devops-agent skill-eval (latest run of each type committed):

Type	Run	Result
structure	v3	12/12 passed (100%)
best-practices	v4, 3 iterations	3/3 iterations passed, 14/17 each (3 asset tests skipped: no assets/), 100% score, 100% consistency
functional	v1, 1 iteration	20/20 runs completed; trigger 10/11 (both negatives correctly did not trigger; verification-unknown-source did not trigger); expected output 7/9 with skill vs 7/9 without (different evals fail on each side); with-skill rated better on all 4 quality pairs judged

evals/evals.json now uses the functional schema: the 9 existing evals unchanged as positive chat evals, plus 2 negative evals.

Skills — complete if this PR adds or changes a skill

• SKILL.md frontmatter is valid: name matches the directory, description present and within length limits, body at most 500 lines.
• The description is written from the agent's point of view — scenarios, services, error types, symptoms.
• Scoped to a specific agent type in metadata.aws-devops-agent-skills.agent-types (INCIDENT_RCA, INCIDENT_TRIAGE, INCIDENT_MITIGATION, …), not left GENERIC without reason.
• Procedures are structured as decision trees on real conditions, not flat prose.
• Reference material lives in references/ or assets/ and loads on demand.
• Non-executable content only — no scripts or code in the skill directory. Deterministic compute belongs in an MCP server.

Security and safety

• Read-only by default; any action that changes customer state is explicit, documented, and approval-gated.
• Customer data stays in the customer's account.
• No destructive operations without an explicit, documented confirmation step.

License confirmation

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

[ams-thakkar]

What needs to change

Eval results are needed, and we're no longer granting the legacy latitude. validate-skill-evals is green here only because aws-routing ships evals.json and eval_queries.json with no committed results, which puts it in the legacy row of the rollout table so violations are reported rather than failed. We're closing that out, so this PR needs structure, best-practices and functional results committed under evals/. I've applied enforce-evals, which is why the check will flip red — that's the requirement being recorded, not a new defect in your change.

We're in the middle of asking authors who were covered by the previous exception state to start using the skill evaluation tool, and since this PR is open it's the right opportunity to do it here.

1.2.4 is already taken, and your checks are stale. main went to 1.2.4 in #152 on 21 Oct, so this needs 1.2.5. The PR is 18 commits behind with mergeable_state: dirty and conflicts in both SKILL.md and CHANGELOG.md. main's changelog already carries a ## [1.2.4] - 2026-10-21 entry for a different change, so the entry needs renumbering as well as the frontmatter — otherwise the file ends up with two different 1.2.4 sections.

Only scan-aws-identifiers and validate-skill-evals have run on this branch. validate-skills, build-docs and CodeQL landed after your last push and will run for the first time on rebase. validate-skills fails on the version until you bump, so rebase and bump in one push.

This reverses a claim the doc previously made, on field evidence rather than a citation. The removed line read - **Multiple DX-GW attachments** (within the same DX-GW): TGW will ECMP across matching prefixes, and the new text asserts the opposite: one DXGW offers a single best path per prefix, so redundant VIFs on one DXGW are active/passive at the TGW. I think you're probably right and the old line was incoherent as written — "multiple attachments within the same DXGW" doesn't describe anything — but the direction of the guidance flips, and the new direction is marked [Field knowledge] rather than sourced. If it's wrong, the skill will tell a customer a design they already run is impossible. This is what the domain SME sign-off is for on this PR specifically: someone who has built TGW-over-DXGW redundancy confirming the one-path-per-prefix behaviour before it ships as guidance.

Nits

Non-blocking.

• The changelog entry is dated 2026-09-21, copied from the 1.2.3 entry above it. The PR was opened 15 Oct.
• The PR description has a stray control character — it reads ^Gws-routing where it should say aws-routing. Only cosmetic, but the description is public.

Thanks

The DX-side contrast is correct and I checked it against the source rather than taking it on trust. The Direct Connect routing and BGP doc says, verbatim: "AWS uses equal-cost multi-path (ECMP) routing across multiple transit or private virtual interfaces when prefixes have the same AS_PATH length and BGP attributes. The ASNs in the AS_PATH of the prefixes do not need to match." You've quoted it accurately and scoped it to the DX side, which is the part that makes the contrast with TGW meaningful.

Marking the unsourced parts [Field knowledge] with the two commands that would confirm them in-account — search-transit-gateway-routes and list-virtual-interface-routes — is the right way to ship an operational observation, and it matches how the doc already handles the 5,000-route selection behaviour. That's what makes the SME question above answerable rather than a matter of opinion.

Flagging the eval-enforcement question yourself rather than assuming was also the right call, even though the answer went the other way.

[ams-thakkar]

Correcting one line above: the check hasn't flipped red, and won't yet.

enforce-evals is applied, but the conflict is blocking the run — GitHub doesn't run pull_request workflows while a PR can't produce a merge commit (mergeable: false). The label takes effect on your first push after the rebase. Requirement is unchanged, just not visible as a red check until then.

[midakin]

Thanks for the thorough review, @ams-thakkar. Update:

Done in this push (6e1360d)

• Rebased onto current main as a single commit; the SKILL.md / CHANGELOG.md conflicts are gone.
• Bumped to 1.2.5 and renumbered the CHANGELOG entry to [1.2.5] - 2026-10-27, so there's no second 1.2.4 section. This also fixes the copied date.
• Fixed the stray control character in the PR description.
• Ran validate_skills.py --base-ref upstream/main --head-ref HEAD locally: 0 errors.

Still open

• Eval results: I'll run the skill evaluation tool and commit the latest structure, best-practices and functional results under skills/aws-routing/evals/ in a follow-up push. I expect validate-skill-evals to go red until then, given enforce-evals.
• SME sign-off: agreed that this flips the direction of the old guidance, so it needs confirmation from someone who has built TGW-over-DXGW redundancy. I'm lining up a reviewer and will attach in-account evidence: search-transit-gateway-routes showing a single DXGW-propagated path per prefix with two VIFs on one DXGW, plus list-virtual-interface-routes from the same setup.

[midakin]

@ams-thakkar - eval results are in (afd6596), run with devops-agent skill-eval and committing only the latest run of each type:

Type	Run	Result
structure	v3	12/12 passed
best-practices	v4, 3 iterations	3/3 iterations passed, 100% score, 100% consistency (BP-13-15 skipped: no assets/)
functional	v1, 1 iteration	20/20 runs completed; skill triggered on 10/11 (both negatives correctly didn't trigger); expected output 7/9 with skill vs 7/9 without, different evals failing on each side; with-skill rated better on all 4 quality pairs judged

Two things worth calling out for review:

• SKILL.md changed in this push, driven by the best-practices findings: the description now opens with "Use this skill when...", reference files are markdown links with a condition for when to load each, and the analysis steps are a checklist with a final self-check step. No technical guidance changed. It's covered by the same 1.2.5 bump and CHANGELOG entry.
• evals/evals.json was converted to the functional schema (skill_name, task_type, should_trigger). The 9 existing evals are unchanged; I added 2 negative evals.

Locally against the head commit: validate_skill_evals.py PASS, scan_aws_identifiers.py 0 findings, validate_skills.py 0 errors. SME sign-off on the one-path-per-prefix behavior is still open.

[github-actions]

This PR is on hold — checklist incomplete

Required items in the PR description are unchecked. A maintainer will not review this PR until each one is checked and actually done.

The check's summary lists them. Complete the work, tick the boxes, and this clears automatically. CONTRIBUTING.md explains each item.