[herosjourney]

Problem

The three infrastructure migration skills design every Redis cache source onto ElastiCache for Redis OSS, while their own Estimate phases already assume Valkey. gcp-to-aws (fast-path.md, database.md), azure-to-aws (fast-path-services.json, index.md, database.md), and heroku-to-aws (design-mapping.md 2C) emit aws_service: "ElastiCache Redis", and heroku-to-aws's cache.tf template hardcodes engine = "redis" with a redis<major> parameter-group family. At the same time ri-sp-eligibility.md, estimate-infra.md, estimate-cost-engine.md, and both estimate-defaults.json files state that Database Savings Plans cover ElastiCache for Valkey only and tell the agent to fall back to Reserved Nodes "because the default mapping is Redis OSS". A customer gets a design that costs 20% more per node than the engine AWS now defaults to, forfeits the Savings Plan the estimate advertises, and sees two phases disagree about what was designed.

Read 2026-11-09:

• Amazon ElastiCache (Valkey) for e-commerce applications: "Use Valkey for new deployments. Valkey is the default engine for new ElastiCache clusters, is fully compatible with Redis OSS commands and data structures, and receives continued development."
• Get started with Amazon ElastiCache for Valkey: node-based Valkey is up to 20% cheaper than other engines and Serverless 33% cheaper; existing Redis OSS reserved-node discounts carry over; zero-downtime in-place upgrade from Redis OSS.
• AWS Price List API (AmazonElastiCache, cacheEngine=Valkey, us-east-1, standard NodeUsage SKUs, via the AWS Pricing MCP server): every Valkey node rate is exactly 0.8× the Redis OSS rate the shared pricing file recorded on 2026-10-05 (e.g. cache.t4g.micro 0.0128 vs 0.016, cache.m6g.large 0.1192 vs 0.149).
• Migrate from Basic, Standard, and Premium tiers to Azure Managed Redis: Azure is moving Azure Cache for Redis customers to Azure Managed Redis, which uses the Microsoft.Cache/redisEnterprise ARM type the azure-to-aws rubric describes only as "Redis Enterprise with modules".

Fix

Make Valkey the designed engine in all three skills and carry the engine through Generate and Estimate. Redis OSS stays available as a recorded exception.

• Design contract. ElastiCache entries emit aws_service: "ElastiCache Valkey" and aws_config.engine: "valkey". "redis" (with aws_service: "ElastiCache Redis") is emitted only when the source depends on a Redis module or a feature Valkey lacks, with the reason in the rationale. azure-to-aws's schema-design-aws.md documents the engine key in its comment (the example object stays {} so the shape contract remains open).
• gcp-to-aws: fast-path.md and index.md rows, and the Memorystore rubric in database.md (Valkey default, Redis OSS exception, Valkey engine_version). Estimate's Price List filter uses cacheEngine=Valkey; the source pricing cache's ElastiCache table carries Valkey rates with the 1.25× Redis OSS note.
• azure-to-aws: Microsoft.Cache/Redis direct row → ElastiCache Valkey; redisEnterprise candidate set and eliminator row name Valkey first, Redis OSS for a Valkey-incompatible feature, and note that Azure Managed Redis uses this type; Q-D6 consequence line; Estimate formula and database-detection rows accept both labels; estimate-defaults.json comments.
• heroku-to-aws: design step 2C records engine: "valkey" and picks a Valkey engine_version (7.2+) rather than copying the source Redis version; redis-elasticache-sizing.json note updated; cache.tf reads engine and family from the design instead of hardcoding redis; generate-docs.md triggers on any ElastiCache* service and says Valkey accepts Redis RDB files and redis-cli; Estimate formula, Price List filter, and estimate-defaults.json comments.
• Shared pricing (shared/pricing/aws-infra-pricing.json, vendored into azure-to-aws and heroku-to-aws): elasticache.nodes now holds the Valkey rates, with engine: "valkey", redis_oss_multiplier: 1.25, and a note on when to apply it.
• Fixtures: azure-iac-terraform after-design / expected-design / after-estimate / expected-estimate and heroku-live-capture/seed-estimate carry the new label and engine; the Azure estimate golden's cache line is repriced at the Valkey rate (24.82 → 19.86) and every dependent figure recomputed (Balanced 465.09 → 460.13, Premium 479.99, Optimized 362.68, deltas, "must_not_be" traps). test_emit_plan_json.py and test_accept_migration_run.py follow.
• New tests/test_elasticache_valkey_default.py pins the three direct mappings, the heroku design/Terraform contract, and the Valkey rates in the shared pricing file.

Left in place on purpose:

• Redis OSS as an option. The eliminator path (modules, Valkey-incompatible features) and the redis_oss_multiplier keep it priceable.
• Data-migration scripts (migrate-redis.sh, redis-cli DUMP/RESTORE). They work unchanged against Valkey.
• prompt-library-for-startups prose that mentions "ElastiCache Redis". Not a migration design surface.
• Memcached mappings (Memcachier) and MemoryDB routing.

Scope

• skills/shared/pricing/aws-infra-pricing.json (+ vendored copies in azure-to-aws, heroku-to-aws)
• skills/gcp-to-aws/references/design-refs/{fast-path,index,database}.md, references/phases/estimate/estimate-infra.md, references/shared/pricing-cache.md
• skills/azure-to-aws/knowledge/design/fast-path-services.json, knowledge/estimate/estimate-defaults.json, references/design-refs/{index,database}.md, references/shared/schema-design-aws.md, references/phases/clarify/clarify-database.md, references/phases/estimate/estimate-infra.md
• skills/heroku-to-aws/knowledge/design/redis-elasticache-sizing.json, knowledge/estimate/estimate-defaults.json, references/phases/design/design-mapping.md, references/phases/generate/{generate-terraform,generate-docs}.md, references/phases/estimate/estimate-cost-engine.md
• fixtures/azure-iac-terraform/*, fixtures/heroku-live-capture/seed-estimate/aws-design.json
• tests/test_elasticache_valkey_default.py (new), tests/test_emit_plan_json.py, tests/test_accept_migration_run.py

No AI/Bedrock catalog change. Does not overlap #598, #583, #585, #588, or #594.

Validation

• mise -C plugins/aws-startup-advisor run ci: all tasks pass (lint:md 0 errors; lint:model-id, lint:model-staleness, lint:vendored-parity 62/62, lint:artifacts 99 artifacts 0 errors, lint:skill-refs, lint:consent-disclosure, lint:startup-model-pricing PASS; test 1414 passed / 222 subtests; test:telemetry 311 passed; test:heroku-source-review pass).
• Valkey node rates pulled from the AWS Price List API through the AWS Pricing MCP server and cross-checked as 0.8× the existing Redis OSS rates for all 13 node types.
• Azure estimate golden recomputed by hand: cache 0.0272 × 730 = 19.86; Balanced 460.13; Premium 460.13 + 19.86 = 479.99; Optimized (324.83 × 0.7) + 135.3 = 362.68; annual 4352.16; savings 3739.87 / 44878.44 / −89.0%.
• Not run: a live Terraform plan of the heroku cache.tf template, and no end-to-end skill run.

Type of Change

Select at least one.

• New skill
• Bug fix
• Documentation update
• CI/CD change
• Other

Startup Advisor review checklist

By submitting this PR, I confirm that:

• I have reviewed the full diff and understand all changes I am submitting.
• I have tested my changes and confirmed they work as intended.
• I have run mise -C plugins/aws-startup-advisor run ci and all checks pass.
• I have disclosed any known issues, limitations, or untested scenarios.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Made with Kiro

[herosjourney]

Reviewed: head 04a5b41 (base 6d6948d), 29 files (+271/−188) — gcp-to-aws, azure-to-aws, heroku-to-aws, skills/shared/pricing. One plugin tree; vendored pricing copies stand in for twins.
Ran: GitHub startup-advisor on this head (pass, 1m44s), plus CodeQL, dependency-review, and build. cmp of skills/shared/pricing/aws-infra-pricing.json against the azure-to-aws and heroku-to-aws vendored copies: identical. Recomputed all 13 shared node rates as exactly 0.8× the 2026-10-05 Redis OSS rates in the same file. Fixture arithmetic: 0.0272 × 730 = 19.86; Balanced 460.13; Premium 479.99; Optimized 362.68; annual 4352.16. Called heroku_conditional_outputs on both labels (result below). Fetched the ElastiCache Valkey e-commerce page (read 2026-11-09): "Use Valkey for new deployments. Valkey is the default engine for new ElastiCache clusters." git merge-tree against upstream/main, #498, and #489: clean.
Did not run: full mise locally (GitHub already ran it), no live Price List query (the pricing MCP server is down), no Terraform plan, no end-to-end skill run.

Cross-plugin parity: PASS for the pricing SSOT — shared file and both vendored copies are byte-identical. gcp-to-aws keeps its own markdown cache (pricing-cache.md); that file is not vendored. This repo has no advisor/migrate twin tree.

Change narrative: Design still emitted Redis OSS while Estimate already assumed Valkey (and Database Savings Plans cover Valkey only) → emit ElastiCache Valkey plus aws_config.engine: "valkey", reprice the node table at 0.8×, and keep Redis OSS as a recorded exception → fast-path rows, heroku design/Terraform/docs, estimate filters, fixtures, and a new text test. Body matches the diff. Prompt-library mentions of "ElastiCache Redis" are left on purpose, as the body says.

Axis	Stance	One line
Parsimony	clear	Label, engine field, rate table, and the fixtures those force.
Completeness	finding	The acceptance gate still matches only ElastiCache Redis.
Security	clear	Markdown, JSON, and a text test. CodeQL and dependency-review passed. No new credential or network surface.
Alternatives	advisory	Keep the Valkey label; the smaller "rename nothing" option leaves the gate green by accident.
Contradictions	finding	Fast-path says Always and, in the same cell, describes an exception.
Overlap	advisory	The 20% / Savings Plan sentence is repeated on every design row. One canonical sentence would do.
Cross-plugin parity	clear	Three pricing files identical.
Propagation & data	finding	Shared node rates check out. The gcp markdown cache's cache.r6g.large row does not, and ri-sp-eligibility.md still describes the old default.

Blocking

1. [Completeness] A Valkey design no longer requires the Redis migration script. scripts/accept-migration-run.py:201 (not in the diff; the label it matches was changed).

   if "elasticache redis" in services:
names.append("scripts/migrate-redis.sh")

   Generate was updated to the new label. skills/heroku-to-aws/references/phases/generate/generate-docs.md:31:

   Any service with aws_service starting with "ElastiCache" (ElastiCache Valkey by default)

   I called heroku_conditional_outputs on this head. {"aws_service": "ElastiCache Valkey"} returns missing: [] (the check PASSes with no script). {"aws_service": "ElastiCache Redis"} returns missing: ["scripts/migrate-redis.sh"]. The test that pins the script, tests/test_accept_migration_run.py:492 and :561, still builds the old label, so CI stays green. The edit at :404 only covers the "not a Postgres script" case.
   Consequence: the default design this PR emits can be accepted without scripts/migrate-redis.sh, which Generate is told to write.
   Fix: match any service whose casefolded name starts with elasticache. Point both tests at ElastiCache Valkey, and add the negative: a Valkey design with no scripts/migrate-redis.sh must FAIL conditional_outputs.

Should-fix

1. [Propagation & data] cache.r6g.large was scaled off a stale, differently named rate. skills/gcp-to-aws/references/shared/pricing-cache.md:214, under a table the intro says was verified 2026-11-09.

   | cache.r6g.large | 0.1648 | 120.30 |

   0.1648 = 0.206 × 0.8. 0.206 is the pre-2026-10-05 cache.m6g.large figure in aws-infra-pricing.json _meta.last_verification (0.206->0.149), not an r6g rate, and r6g is not one of the 13 node types that file repriced. The other six rows in this table do match the shared Valkey rates, and their monthly figures match hourly × 730 (19.86, 9.34, 18.69, 37.96, 120.30).
   Fix: replace the row with a Price List cacheEngine=Valkey rate for the node you mean, or delete it. Do not ship 0.8 × 0.206 under the name r6g.

2. [Contradictions] The Memorystore fast-path row is both unconditional and conditional. skills/gcp-to-aws/references/design-refs/fast-path.md:53.

   | google_redis_instance | ElastiCache Valkey | Always | … Redis OSS only when a Redis module or feature Valkey lacks is in use |

   Always means the row matches and the rubric is skipped. The same cell tells the agent to emit Redis OSS in a case the condition cannot express. database.md states the exception separately, and that section is background, not the routing row.
   Fix: change the condition to no Redis module and no Valkey-incompatible feature, and on that miss send the agent to the database.md exception. Or drop the exception from this cell if Memorystore cannot surface one.

3. [Propagation & data] The commitment SSOT still says the default engine is Redis OSS. skills/shared/estimate/ri-sp-eligibility.md:40, and the gcp / azure / heroku vendored copies (not in the diff). Estimate phases this PR edited point here.

   This matters directly for any skill whose default target engine is Redis OSS

   heroku estimate-cost-engine.md now says the default is Valkey and that ElastiCache normally belongs on the Database Savings Plan. An agent that treats this sentence as the default falls back to Reserved Nodes for the design this PR emits.
   Fix: The three migration skills default to Valkey, which is the row above. Apply this Redis OSS row only when Design recorded aws_config.engine = "redis" or the engine is Memcached.

Nits

fixtures/azure-iac-terraform/after-design/aws-design.json component rationale still says "one ElastiCache Redis cluster" while the service entry on the same artifact is ElastiCache Valkey.

Heads-up (out of diff, pre-existing — not blocking)

ri-sp-eligibility.md:39 says Database Savings Plans are "the only lever for serverless ElastiCache, regardless of engine." Line 41 says Redis OSS / Memcached serverless has no commitment product. Those two sentences cannot both be followed. Worth a follow-up on the eligibility file; this PR does not introduce it.

Alternatives (max 2)

1. Keep the service label ElastiCache Redis and only add engine: "valkey" — wins: the acceptance gate keeps firing; costs: every reader of aws_service still thinks the engine is Redis OSS, which is the bug the PR is fixing. Keep the new label and update the gate.
2. Leave node rates at the Redis OSS numbers and apply × 0.8 in the formula — wins: one multiplier instead of a rewritten table; costs: every cached total drifts until the formula is applied, and the file stops being a rate card. Keep the repriced table.

Overlap

merge-tree against #498 and #489 is clean. #489's render-to-aws vendors aws-infra-pricing.json at the Redis OSS rates (cache.t4g.micro 0.016, no engine field). If #489 lands second it reintroduces that table in a new skill. Rebase #489 onto this head so its vendored copy is the Valkey file.

Verdict

Request changes — one blocking item: the default Valkey design passes acceptance without the migration script Generate is told to write. Commenting rather than requesting changes: this review is posted from the PR author's account (herosjourney). Code owners (Agent Toolkit Admins) decide the merge.

[herosjourney]

Re-review at 4f57512 (previous: 04a5b41). Delta: 1 commit, 8 files. GitHub startup-advisor passed on this head. Re-called heroku_conditional_outputs on this head. Confirmed cache.r6g.large against public on-demand tables (Valkey NodeUsage $0.1648, Redis NodeUsage $0.206, us-east-1). Did not re-query the Price List API.

Prior finding	Status at head
Blocking — Valkey design skipped scripts/migrate-redis.sh	Fixed at 4f57512. The predicate is service.startswith("elasticache"). A Valkey design and a Redis design both return FAIL with that script missing. The new test covers both labels.
Should-fix — cache.r6g.large looked like a scaled stale m6g rate	Withdrawn. The row now cites a Valkey NodeUsage read, and public tables agree: Valkey $0.1648, Redis OSS $0.206. The shared m6g.large figure of $0.206 was a coincidence.
Should-fix — fast-path row was both Always and conditional	Fixed at 4f57512. The condition is "No Redis module and no Valkey-incompatible feature", and a miss points at database.md.
Should-fix — ri-sp-eligibility.md still said the default engine is Redis OSS	Still open on the copy gcp-to-aws loads. Shared, azure-to-aws vendored, and heroku-to-aws vendored are fixed. skills/gcp-to-aws/references/shared/ri-sp-eligibility.md:40 still says "any skill whose default target engine is Redis OSS". estimate-infra.md tells the agent to read that file, and that it governs on a disagreement.
Nit — fixture rationale still said "ElastiCache Redis cluster"	Fixed at 4f57512.

New in the delta: the widened predicate also matches Memcached. Sweep of startswith("elasticache") against designs this repo emits: ElastiCache Memcached (Memcachier, fast-path-addons.json).

Should-fix

1. [Completeness] The gcp copy of the commitment file still describes the old default. skills/gcp-to-aws/references/shared/ri-sp-eligibility.md:40.

   This matters directly for any skill whose default target engine is Redis OSS

   The shared file and the azure/heroku vendored copies now say the three skills default to Valkey. gcp-to-aws Estimate loads this copy, not those.
   Fix: the same sentence now in skills/shared/estimate/ri-sp-eligibility.md:40.

2. [Completeness] A Memcachier design now fails acceptance until someone writes a Redis script. scripts/accept-migration-run.py (the new startswith("elasticache") check). I ran it: ElastiCache Memcached returns FAIL, missing scripts/migrate-redis.sh.
   generate-docs.md uses the same prefix for has_redis, so Generate writes that script and a "Redis Migration" section for Memcached too. Memcachier is a different protocol; the PR left that mapping alone on purpose, and Generate already lists ElastiCache Memcached as a no-Terraform warning rather than a Redis migration.
   Fix: match elasticache valkey and elasticache redis only, in the gate and in the has_redis trigger (generate-docs.md, both places that say "starting with ElastiCache"). Add a Memcached case that must PASS without migrate-redis.sh.

Verdict

Comment — supersedes the request-changes on 04a5b41. That blocking finding is fixed and re-probed: a Valkey design now fails closed without the migration script. Two should-fix items remain (the gcp eligibility copy, and Memcached inheriting the Redis script). Commenting rather than approving: this review is posted from the PR author's account (herosjourney). Code owners (Agent Toolkit Admins) decide the merge.

[herosjourney]

Re-review of #599 at 098c8ee (delta since 4f57512: one commit, 5 files)

Ran: diff 4f57512..098c8ee; tests/test_accept_migration_run.py (49 passed, 96 subtests); tools/sync-vendored.py --check (62 vendored files byte-identical); a direct probe of heroku_conditional_outputs; CI on this head (all 10 checks pass).
Did not run: a live AWS Price List query (the pricing MCP servers were unavailable).

Prior findings

Finding	Status
Should-fix: gcp-to-aws/references/shared/ri-sp-eligibility.md:40 still said Redis OSS is the default target engine	Fixed at 098c8ee. The row now says the three migration skills default to Valkey (the row above) and tells the reader to apply the Redis OSS row only when Design recorded aws_config.engine = "redis" or the engine is Memcached.
Should-fix: accept-migration-run.py required scripts/migrate-redis.sh for any aws_service starting with elasticache, so Memcached (Memcachier) failed the gate	Fixed at 098c8ee. The gate now matches only elasticache valkey and elasticache redis. Probe results: Valkey FAIL (script missing), Redis FAIL (missing), lowercase elasticache valkey FAIL (missing), Memcached PASS.
Same Memcached over-match in generate-docs.md (has_redis, lines 31 and 1635)	Fixed at 098c8ee. Both triggers now name Valkey and Redis explicitly, and say Memcached gets no Redis script.

New in delta

• Nit (non-blocking) — tests/test_accept_migration_run.py, test_memcached_design_does_not_require_the_redis_migration_script: the test only asserts assertNotIn("scripts/migrate-redis.sh", result["detail"].get("missing", [])). It does not pin result["status"]. If the Memcached case failed for an unrelated reason, missing would still not contain the script and the test would pass. I probed the real result (status == "PASS", missing == []), so behavior is correct today. Suggested fix: add self.assertEqual("PASS", result["status"]).
• generate-terraform.md Step 8 now lists exactly Valkey and Redis, and says Memcached stays a no-Terraform warning. That is consistent with line 2485 of the same file, which already lists Memcached as an unsupported mapping.

Sweep

• Searched non-vendored files for starting with "ElastiCache", startswith("elasticache") and starts with "ElastiCache". No prefix match remains.
• Searched every consumer of has_redis and migrate-redis. generate-assemble.md:220 and generate.md:82 say "when Redis in design", which is consistent with the narrowed match.

Verdict (supersedes the 05:00Z review)

No blocking or should-fix items are open at 098c8ee, and CI is green. The one nit can ride along or be a follow-up. This is posted as a COMMENT review from the author's account (herosjourney), so it cannot be an approval. The merge decision is for Agent Toolkit Admins.

One note from earlier still applies: #489's render-to-aws vendors the old Redis-rate aws-infra-pricing.json, so it should be rebased onto this PR after merge.

[leon1418]

[🤖 AI review 🤖]

Reviewed 098c8eee1765b6e10d6e60faa895c73706296796: all 34 changed files, OCR delegation, repository review, and one fresh independent general reviewer. Three P2 correction areas remain:

1. Preserve Memcached prices in the shared rate card and its Azure/Heroku vendored copies.
2. Reconcile the Azure estimate golden, cache acceptance band, and per-cluster amounts.
3. Keep the Heroku engine, supported version, and service label consistent for the default, explicit Redis OSS choice, saved-design resume, and seed fixture.

The Memcached migration-script over-match is fixed: both Redis-compatible labels require the script, while Memcached passes without it. The inline comments below contain the remaining correction paths and acceptance criteria.

Validation: 62 focused tests and 96 subtests passed; all 10 listed GitHub checks passed. The original Azure estimate fixture checker failed on the two arithmetic/band issues below. Original-source probes verified the cache predicates and all 13 Valkey rates against captured AWS Price List data. No live Terraform plan or end-to-end migration was run.

[leon1418]

[🤖 AI review 🤖]

Verified the fixes at 1b7bffcf04557f0d1bc050cf09f987974bc38215. All three prior findings are fixed: Memcached pricing, Azure fixture arithmetic, and Heroku engine/version/legacy-resume handling. Evidence replies are in the original conversations, which are now resolved.

Completed OCR delegation, repository review, and one fresh independent general review across all 34 changed files and affected consumers, including the GCP billing-only route. The combined follow-up gate passed with no remaining findings.

Validation: 62 focused tests and 96 subtests passed; the original Azure fixture checker and all 62 vendored-file parity checks passed. Pricing checks matched 39 engine/node combinations against captured AWS Price List data. All ten GitHub checks passed at this head. No live Terraform plan or end-to-end migration was run.

Approval is recommended for the user. This is a COMMENT review; the earlier blocking review has not been dismissed.