fix(gcp-to-aws): withhold Discover cost range when Terraform sizes exceed preview defaults

[herosjourney]

Problem

Discover's migration preview always prices PRIMARY resources at a hardcoded development-tier stub (0.5 vCPU Fargate, db.t4g.micro / 20 GB single-AZ, cache.t4g.micro, 2× t4g.small EKS nodes) and prints that sum as AWS cost (rough) with a ±30% label.

On production-sized Terraform that is a trust failure. A stack with db-custom-32-122880 REGIONAL HA + 2 TB, Memorystore STANDARD_HA 100 GB, Cloud Run min 50 × 8 vCPU, and 6× e2-standard-16 GKE nodes is a ~$44k–$54k/mo GCP bill; the preview still quotes ~$175–$263/mo. Users compare that to their real bill (or COST_ESTIMATE.md) and conclude the tool is broken.

±30% does not cover an order-of-magnitude miss. The stub map is still useful for tiny stacks; it must not be the headline when authored sizes clearly exceed it.

Solution

Add an authored-size gate to discover-preview.md (infra route Step 2) in both plugin copies:

• Scan inventory config for every type in the gate table, including SECONDARY. Instance templates, MIGs, and Dataflow jobs are not Priority-1 PRIMARY; a PRIMARY-only scan leaves those rows dead letter.
• If any signal fires (Cloud Run min instances > 1 / CPU > 1 / memory > 2Gi; Cloud SQL not db-f1-micro/db-g1-small, REGIONAL, disk > 20 GB, or replicas; Redis > 1 GB or HA; GKE/GCE/instance-template machine type not *micro*/*small* or node count > 2; MIG target_size > 2 or linked autoscaler min_replicas > 2; any google_dataflow_job; Filestore/Spanner above the stub):
  • Do not compute or store the stub sum.
  • Set cost_preview.aws_monthly_range_usd to null, quote_suppressed: true, quote_suppressed_reason: "authored_sizes_exceed_preview_defaults", and up to 5 authored_size_signals in fixed type order (Cloud SQL → Redis → GKE/node pool → Cloud Run → Dataflow → MIG/template → others).
  • Chat COST_ROW becomes: Not quoted at Discover — Terraform sizes are above the preview defaults… Full AWS number in Estimate after you confirm sizing. Real GCP spend from billing is still shown when present.
  • disclaimer is required on suppress (gate sentence, not the stub ±30% line).
• Tiny stacks that stay inside the stub (Cloud Run 1 instance, db-f1-micro, etc.) keep the existing ~$X–$Y range.

report-decision-core.md (both copies): if only the preview exists and the quote is suppressed, do not render a dollar headline or "Early estimate (±30%)". Render the withheld-at-Discover sentence instead. Estimate artifacts still supersede the preview.

No JSON Schema change: tsc --noEmit is the frontmatter validator; the preview contract lives in the markdown field list.

Type of Change

• Bug fix
• New plugin/power/tool
• Enhancement to existing content
• Documentation update
• Guardrail/CI update

Team Folder

• advisor/
• migrate/
• solution-architecture/
• Other: ___

Scope

In: Discover preview cost row + migration-preview.json cost_preview contract; decision-report cost headline when Estimate has not run.

Out: Changing Design/Estimate defaults (still development-tier unless Clarify keeps authored sizes). Pricing the authored production stack at Discover time. Heroku Discover (no equivalent stub-dollar preview). Adding a JSON Schema for migration-preview.json.

Verification

• mise run lint:md: PASS (0 errors / 874 files)
• mise run fmt:check: PASS (dprint fmt applied to the two discover-preview.md copies)
• mise run drift:check: OK (273 identical / 25 allowlisted)
• Manual: large-scale Cloud SQL / Redis / Cloud Run / GKE fixture now writes quote_suppressed: true and aws_monthly_range_usd: null instead of ~$175–$263
• Follow-on: SECONDARY GCE/Dataflow types are now in the scan set so added table rows are reachable
• mise run build security scanners (bandit / semgrep / gitleaks / checkov / grype) not re-run locally — markdown-only change; CI will cover them

Checklist

• I have read the CONTRIBUTING.md guidelines
• My changes do not include hardcoded secrets, credentials, or internal-only content
• I have run mise run lint:md and mise run fmt:check locally and they pass (full mise run build security suite left to CI; markdown-only)
• I have updated documentation if needed
• My changes are scoped to my team's folder only (both advisor/ and migrate/, per the drift-gate mirroring requirement)

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 Cursor

[leon1418]

[🤖 AI review 🤖]

Reviewed: all 4 changed files (+84/−8), head 10dd1f9.

Cross-plugin parity: discover-preview.md — byte-identical (blob ac570aa2) ✅. report-decision-core.md — different blob SHAs, but the sole divergence is the pre-existing fixture path on line 40 (advisor/plugins/... vs migrate/plugins/...); the cost-headline change on line 63 is identically worded in both copies ✅. Drift allowlist: 0 changes ✅.

Design: Sound. The authored-size gate addresses a real trust failure — quoting dev-tier stub dollars against production-sized Terraform is misleading by an order of magnitude. The gate is properly a hard pre-check before any dollar range is written, not a post-hoc disclaimer. The field contract (quote_suppressed / quote_suppressed_reason / authored_size_signals / disclaimer) is cleanly separated from the existing aws_monthly_range_usd path so both consumers (the chat COST_ROW template and report-decision-core.md) can branch on a single boolean. Scanning SECONDARY types for instance templates, MIGs, and Dataflow is correct — these are not Priority-1 PRIMARY but still carry sizing information that would blow past the dev-tier stub.

Functionality: Gate table thresholds are well-chosen for their purpose (detecting "clearly not a toy stack"). The fixed type ordering for authored_size_signals prevents non-deterministic output. The max-5 cap is reasonable for chat display. The report-decision-core.md change correctly falls through to the existing "Early estimate (±30%)" path only when the quote is not suppressed, and correctly refuses to render dollar headlines when it is.

Nit: The Step 5 JSON example only demonstrates the non-suppressed case. A second example (or a comment block) showing the suppressed shape (aws_monthly_range_usd: null, quote_suppressed: true, authored_size_signals: [...]) would make the contract more copy-pasteable for someone implementing a consumer. The field rules below the example do cover the suppressed case fully, so this is documentation polish, not a correctness gap.

Tests/CI: CI 8/8 SUCCESS (build, gitleaks, bandit, semgrep, checkov). mise run lint:md and mise run fmt:check reported PASS per the PR description. mise run drift:check OK (273 identical / 25 allowlisted). Markdown-only change — no runtime code to unit-test.

Result: CLEAN — 0 mandatory blockers, 1 Nit (JSON example completeness). MERGEABLE pending maintainer approval.

[herosjourney]

Addressed the nit in 9b5e126.

Added a suppressed-quote example to Step 5 in both discover-preview.md copies, immediately after the existing non-suppressed (stub-range) example. It demonstrates the full suppressed shape — aws_monthly_range_usd: null, quote_suppressed: true, all five authored_size_signals slots filled in fixed type order, and the gate disclaimer (not the ±30% line). Uses a realistic large-stack fixture (Cloud SQL REGIONAL, Redis 100 GB HA, GKE e2-standard-16, Cloud Run min 50) to make the contrast with the toy-stack example obvious.

Files are byte-identical; drift:check is OK (273 identical / 25 allowlisted). fmt:check passes. The existing lint:md error is pre-existing on this branch (unrelated file in migration-watchdog/).

[herosjourney]

Fixed in 609773f — gitleaks was pattern-matching on the field=value format in the authored_size_signals example strings (tripping the generic-api-key rule). Switched to space-separated "address: field value" format throughout — the example block and the step-3 prose. No semantic change.

[leon1418]

[🤖 AI review 🤖]

Follow-up review of 456b427: the suppressed-quote example is now present. A closer check against the Terraform Google provider definitions found three gaps in the gate, so this updates my earlier CLEAN assessment.

The inline comments cover Cloud Run v1 minimum-instance annotations, GKE node-count fields, and inconsistent Spanner capacity thresholds. Each case can let a deployment above the intended preview limits retain the development-tier quote. Please address these in both plugin copies before merging and add regression cases for the relevant field variants, including equivalent Spanner capacities expressed in nodes and processing units.

Validation: reviewed all four changed files, traced inventory extraction and report consumers, and checked the provider/API definitions. Both plugin copies have equivalent changes, and the current commit has eight successful CI checks. I did not run an end-to-end LLM migration; these findings come from checking the specified rules against valid input configurations.

[herosjourney]

Fixed in 2e5414f. All three edge cases confirmed real:

1. Cloud Run v1 minScale annotation (P2) — added a normalization note in discover-preview.md explaining v1 has no min_instance_count field (it's template.metadata.annotations["autoscaling.knative.dev/minScale"] or the service-level run.googleapis.com/minScale), and added a matching Cloud Run extraction block to discover-iac.md Step 1 — same pattern as the existing Cloud SQL normalization block — so config.min_instance_count is always populated at a canonical name regardless of API version.

2. GKE fixed node_count / total_min_node_count / total_max_node_count (P2) — extended the gate condition to cover all three sizing forms (fixed count, per-zone autoscaling, cluster-wide autoscaling) and added the corresponding discover-iac.md extraction fields.

3. Spanner num_nodes vs processing_units (P2) — added an explicit normalization step (1 node = 1,000 PUs, so divide processing_units by 1000) before the threshold comparison, so num_nodes = 1 and processing_units = 1000 evaluate identically.

Since this is a markdown instruction spec with no executable test harness, I added a "Worked normalization cases" table right after the gate table pinning the expected fire/no-fire outcome for each variant (v1 annotation forms, all three GKE sizing forms, and the Spanner unit-equivalence case) — this is the regression-coverage equivalent for a spec file.

Both copies stay byte-identical. fmt:check clean, drift:check OK (273/25), lint:md shows only the pre-existing unrelated error in migration-watchdog/.

[leon1418]

[AI review]

Re-reviewed 50421e6, including the fixes described in 2e5414f and all six changed files.

The original Cloud Run v1 annotation examples and standalone GKE fixed/total-count examples are now covered. Spanner's equivalent node/PU inputs also produce the same result under the revised >1-node threshold. The worked cases make these expectations clearer.

Three normalization gaps remain, detailed inline: Cloud Run v2 revision-level minimum instances, GKE node pools declared inside the cluster resource, and per-zone GKE counts being compared directly with total counts. These are valid Terraform configurations that can still retain the development-tier quote despite exceeding the intended preview limits. Please extend the extraction rules, gate, and worked cases in both plugin copies to cover them.

Validation: checked the official Google provider documentation and GKE provider schema; verified that both discover files are byte-identical across plugins and that the report copies differ only in their expected fixture path. The current head has eight successful CI checks. This was a specification/source review, not an end-to-end LLM execution. Keeping Request changes for the remaining cases.

[herosjourney]

Addressed all three remaining normalization gaps in 675a73b. All three were valid — each is a real Terraform provider path that let a production-sized stack keep the development-tier preview quote.

1. Cloud Run v2 revision-level scaling. A v2 service can set its minimum on template.scaling.min_instance_count (revision-level), not just service-level scaling.min_instance_count. Added to the extraction rule (discover-iac.md), the preview-gate normalization bullet, and a worked case (template.scaling.min_instance_count = 50, no service-level scaling → fires).

2. Inline GKE node_pool blocks. google_container_cluster can declare pools inline as node_pool { ... } blocks rather than standalone google_container_node_pool resources. Inline blocks aren't separate Terraform resources, so a per-resource scan missed a 20-node inline pool. Extraction now traverses every inline pool (capturing machine_type, counts, and node_locations), the gate evaluates each pool standalone-or-inline, and a worked case covers the inline 20-node pool.

3. Per-zone vs total node counts. min_node_count/max_node_count are per-zone while total_* are pool totals; the gate compared both against the same >2 threshold. It now converts per-zone counts to totals (per_zone × node_locations zone count) before comparing. I also corrected the previously unsafe max_node_count = 2 → No worked case (now explicitly single-zone) and added a three-zone 2 × 3 = 6 → Yes case.

Applied identically across both plugin copies (byte-identical, verified). cross-plugin-drift OK (273 identical); dprint check clean.

Note on the "regression cases" request: this authored-size gate is specified through the worked-case tables rather than an executable fixture harness (there's no test suite wired to the preview gate, unlike the report validator), so I added the cases there — consistent with how the earlier rounds were addressed in 2e5414f. Happy to wire executable fixtures if you'd prefer that as a follow-up.

[leon1418]

[AI review]

Re-reviewed 675a73b against 50421e6.

Confirmed that Cloud Run v2 revision-level minimum instances and inline GKE node-pool extraction are now covered in the extraction rules, gate, and worked cases. The explicit three-zone autoscaling case is also fixed.

One P2 remains in the existing GKE normalization discussion: the new rule incorrectly treats fixed node_count / initial_node_count as pool totals. They also need zone normalization for regional or multi-zonal pools, so a fixed two-nodes-per-zone pool across three zones still retains the two-node preview quote. I replied in that thread with the provider field semantics and the two missing worked cases.

The two discover files remain byte-identical across plugins; the report copies differ only in their expected fixture path. This was a specification/source review, not an end-to-end LLM run. The worked-case tables are sufficient for this scoped fix; I am not requesting a new executable harness as a merge prerequisite.

Keeping Request changes for that remaining correction.

CI at review time: 7 successful, 1 pending, 0 other completed checks.

[herosjourney]

Addressed the remaining P2 in de0d3fb — you're right, and the provider semantics you cited are correct.

The prior fix wrongly treated fixed node_count / initial_node_count as pool-wide totals. Per the Google provider they're per-zone (node_count per instance group, initial_node_count per zone including the cluster default pool), so a regional e2-small pool with node_count = 2 across three node_locations is 6 nodes yet kept the two-node preview quote.

Fix:

• The GKE normalization bullet now states that everything except total_min_node_count / total_max_node_count is per-zone, and applies the × node_locations zone multiplier to node_count, initial_node_count, min_node_count, and max_node_count. Only the total_* fields are used unchanged.
• Updated the GKE gate row to match.
• Added the two worked cases you asked for: fixed node_count = 2 across 3 zones (= 6 → fires) and initial_node_count = 2 across 3 zones (= 6 → fires). Clarified the existing node_count = 20 case as single-zone (20 → fires).

Kept the worked-case approach per your note that an executable harness can be a follow-up. Both plugin copies updated in lockstep (byte-identical, verified); cross-plugin-drift OK (273 identical); dprint check clean.

[leon1418]

[AI review]

Re-reviewed 80fbf35, including the final correction in de0d3fb. The remaining GKE finding is addressed: node_count, initial_node_count, min_node_count, and max_node_count all receive the zone multiplier, while total_min_node_count / total_max_node_count remain unchanged. Both requested fixed-count examples now explicitly evaluate to six nodes across three zones and suppress the quote.

The earlier Cloud Run v1/v2 extraction, standalone and inline GKE pool handling, and Spanner unit-equivalence fixes remain intact. I have no remaining blocking findings from this review and approve this scoped fix.

Verification: all three changed file pairs are byte-identical across the advisor and migrate plugins, and all nine checks on the current head have passed. This was a specification/source review, not an end-to-end LLM migration run. The worked cases are sufficient for this PR; an executable preview harness can remain a follow-up.