Repository navigation
feat(aws-startup-advisor): add render-to-aws migration skill - #489
Open
herosjourney wants to merge 26 commits into
Open
herosjourney wants to merge 26 commits into
herosjourney wants to merge 26 commits into
Conversation
Add a Render-to-AWS migration skill alongside heroku-to-aws, gcp-to-aws and azure-to-aws. It runs the shared Discover -> Clarify -> Design -> Estimate -> Generate -> Workshop -> Feedback pipeline on a Render workspace and emits the same deterministic artifact set. - Discovery covers the six Render service types (web, private, static, background worker, cron job, workflow) plus Render Postgres, Key Value, persistent disks, Build Sources and Workflows. Inputs are a Blueprint (render.yaml validated against Render's published schema under references/render/), Render CLI 2.30.0 JSON captures, and GET-only REST projections; every capture is validated against the contracts under references/shared/ and secret fields are rejected by negative fixtures. - Design uses exactly-one AWS selectors per Render resource (compute, data, storage, delivery, networking, workflows) backed by the knowledge/design sizing tables, with negative selector cases as fixtures. - Generate produces the migration guide, report and scripts; Terraform generation is opt-in via preferences and the expected output is checked in under fixtures/expected/generated/terraform. - scan-render-application.py emits an ai-workload-profile.json that conforms to the aws#415 shared contract (scripts/contracts/ ai-workload-profile.schema.json, mirrored byte-identically at references/contracts/ and checked by the plugin tests). AI execution is deferred (deferred_shared_ai_engine) until bedrock-migration-assessment consumes aws#415; the skill does not run an AI migration itself. - Register the skill in the plugin manifests (including the Devin manifest), marketplaces, README and tools/validate_spec.py; register Render's artifacts in scripts/artifact-contracts.json (fixture prefix render-, inventory, clusters, ai-discovery, preferences, aws-design, estimation-infra, generation-warnings) and add render-to-aws to validate-artifacts.py --skill; vendor the shared DSL, pricing, estimate, clarify, state and workshop files byte-identically with a references/vendored/README.md manifest for lint:vendored-parity; pin jsonschema 4.23.0 and PyYAML 6.0.2 in the root lint:spec task and the plugin test task. - 142 Render tests: script unit tests under skills/render-to-aws/scripts and plugin-level phase, fixture and ai-profile contract tests driven by tests/render_pipeline_harness.py. Plugin CI (mise -C plugins/aws-startup-advisor run ci) passes with 553 tests.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
added 3 commits
October 22, 2026 11:43
…gs in render-to-aws - parse-render-billing.py: stop echoing the source digest on stdout. CodeQL flagged the success line as clear-text logging of sensitive data (py/clear-text-logging-sensitive-data). The digest is a SHA-256 of the user-supplied billing export, not a credential, but nothing derived from that file needs to reach stdout; it is already persisted as source_digest in the written artifact. The ABI doc is updated to match. - Bump the skill-local test dependency pytest 8.3.5 -> 9.0.3 to clear GHSA-6w46-j5rx-g56g (vulnerable tmpdir handling, moderate). Test-only; never runs in a user migration. uv.lock regenerated (adds pygments as a pytest 9 transitive). Pin assertion in test_render_phase_contracts updated. Render suite 142 passed under pytest 9.0.3; plugin CI 553 passed; root build (manifests, spec, markdown, gitleaks) clean.
…e Render replay validates in CI tests/render_pipeline_harness.py proves each generated migration pack is deployable by running `terraform fmt`, `init -backend=false` and `validate` against it, and fails closed if the binary is absent. The GitHub runner has no Terraform, so the Startup advisor workflow failed on test_complete_mocked_pipeline_replays_all_phases_from_same_inventory. Declare terraform = "1.15.2" under [tools] in the plugin mise.toml. The workflow already runs jdx/mise-action with working_directory set to the plugin (aws#423), so it installs whatever this file declares; no change to .github/ or the root mise.toml. gitleaks is already inherited from root. Pinned to the exact version the pack was validated with locally so CI and local runs check identical Terraform behavior. Verified by running `mise -C plugins/aws-startup-advisor run ci` with terraform masked from PATH: 553 passed, mise supplied Terraform v1.15.2. README Development checks section updated per the aws#423 extension contract.
… with Apache-2.0 asyncpg dependency-review rejected psycopg==3.2.3 (LGPL-3.0-only) in skills/render-to-aws/fixtures/render-application/requirements.txt. That file is a synthetic Render app the scanner reads to prove it detects a Postgres client; nothing installs it. The finding was masked on the first CI run by the pytest advisory and surfaced once that was fixed. Swap the fixture dependency to asyncpg==0.30.0 (Apache-2.0) and add "asyncpg" to the postgres_client detector alongside psycopg/pgx/sequelize so the fixture still exercises Postgres detection. Verified the scanner CLI on the fixture still emits postgres_client with no psycopg residue; Render suite 142 passed, plugin CI 553 passed.
leon1418
requested changes
Oct 22, 2026
leon1418
left a comment
leon1418
left a comment
Contributor
There was a problem hiding this comment.
[🤖 AI review 🤖]
Requesting changes at 1aee9d5f1b21d27a1aefac40969a7739abd37a2f. The completed review produced this consolidated correction list; each inline comment includes its supported failure case and correction.
- Give Discover an execution scope that can run its required Python helpers.
- Preserve permitted metric/log values and complete metric observation history.
- Handle reviewed wrapped-null CLI responses.
- Accept the helper's reviewed HTTP 404 absence shape in its schema.
- Scope environment aliases by project.
- Remove obsolete AI files on successful non-AI rediscovery.
- Exclude generated migration output from application scans.
- Attribute AzureOpenAI call/model evidence to the correct source service.
- Support no-AI and unavailable-price report inputs.
- Keep prorated billing totals invariant under period splitting.
- Apply the documented default for omitted Blueprint disk size.
- Align equivalent published plan names with sizing and pricing aliases.
- Avoid immutable promotion-tag collisions between distinct images.
- Generate the selected Valkey engine.
- Reject non-connectivity Terraform initialization failures in the replay.
Validation: 655 parent tests and 142 independent-reviewer Render tests passed, including two-run generated-pack replays with local Terraform, policy, schema, report, secret-scan and byte-equality checks. Separate probes reproduced the cases above. HTTP/AWS/error-response simulations are labelled explicitly; no infrastructure was deployed. Current-head startup-advisor, build, dependency-review and CodeQL checks succeeded at the latest capture; get-pr-info remained queued.
added 7 commits
October 22, 2026 17:37
…CLI/REST shapes and project-scoped identities Addresses findings 2, 3, 4, 5 and 6 of leon1418 review 5620071586 on PR aws#489. Every root cause was reproduced before the fix. - Finding 2: `_safe_tree` applied SENSITIVE_KEY to every nested key, so the `value` keys of populated REST metric series and CLI log labels raised `secret_bearing_input`, and `lifecycle[parent].metrics[category]` was overwritten per capture so only the last 24h window survived. Add `_metric_series_projection` / `_log_metadata_projection` (exact closed shapes, NFC, string-level secret checks plus `rnd_` literals; any other shape is `metric_series_invalid` / `log_metadata_invalid`), retain every window as a list of {outcome, window, captured_at, source_ref, series} sorted by (start, end, captured_at), and reject duplicate windows as `duplicate_metric_window`. All other keys keep `_safe_tree`. - Finding 3: `_iter_cli_data` checked for null before unwrapping the `{"data": ...}` envelope, so the reviewed `data_null` shape of postgres_get/keyvalue_get became `capture_data_invalid`. Unwrap first; null yields no resource row only for categories whose contract shape in render-cli-contracts.json is nullable, everything else still fails. - Finding 4: add `http_404` to `raw_shapes` in schema-render-rest-projections.json so the reviewed absence envelope validates; the status enum stays closed. - Finding 5: environment aliases were a global name -> ID map, so two projects each owning `production` raised `ambiguous_membership_identity`. Key aliases by (resolved project, alias), resolve candidates with their project context (unqualified only when exactly one project owns the alias), and match Blueprint environments to live ones per project. Duplicates within one project are still rejected. - Finding 6: `--output-ai-profile` / `--output-ai-discovery` are now required and both always join the `_write_generation` transaction, so a non-AI rerun removes stale AI artifacts atomically and a failed rerun rolls back all four files. The `ai_output_required` branch is gone and script-abis.md documents the required flags. New tests cover populated metric/log handoff, seven-window retention, secret rejection outside the closed shapes, nullable CLI envelopes with non-nullable controls, same-named environments across projects (live-only and mixed) with the same-project duplicate still rejected, the REST absence envelope against the published schema with http_500/unknown status rejected, and the AI cleanup/rollback paths. Verification: the FEAT pytest set (test_normalize_render_capture, test_capture_render_api, test_render_fixture_contracts, test_render_ai_profile_contract) passes 90 tests; lint:artifacts passes.
… and bind AI evidence to its client Addresses findings 7 and 8 of leon1418 review 5620071586 on PR aws#489 against scan-render-application.py. Both root causes were reproduced before the fix. - Finding 7: `.migration` was not in EXCLUDED_DIRS and scan() never learned the configured --migration-dir, so retained run artifacts such as `.migration/<run>/application.json` (which keep `OpenAI(` and `responses.create` as evidence symbols) were rescanned as application source and re-emitted an AI profile on their own. `.migration` is now excluded by name, scan(root, migration_dir) also skips the resolved migration dir when it lies under the workspace root, main() passes it, and the skip is recorded in skipped[] as `excluded_directory`. - Finding 8: import/sdk_construction/method_call needles were matched as case-insensitive substrings, so `OpenAI(` fired inside `AzureOpenAI(` and `from openai` inside `from openai import AzureOpenAI`, while every gpt-*/o*-* model was attributed to provider `openai` regardless of the client the file constructs. Needles are now identifier-bounded regexes (the direct `from openai import` additionally excludes Azure imports), `chat.completions.create` is a shared method call bound to the client constructed in the same file (direct owner when none is), and OpenAI model identifiers bind to the client(s) constructed or called in that file, falling back to the provider-wide owner otherwise. An Azure-only file now yields a single azure_openai source carrying its model and no phantom openai_api source; a mixed direct+Azure file keeps both. Tests cover a rescan with retained output under `.migration` and under a custom migration dir (plus positive controls), the Azure-only file, the mixed file, a direct-only multi-file app (no azure source), and keep the fixture (gpt-4o-mini on openai_api) and OpenRouter cases green. Verification: scanner + normalizer + ai-profile + fixture pytest set 72 passed.
…regate billing intervals, default disk size Addresses findings 9, 10 and 11 of leon1418 review 5620071586 on PR aws#489. - Finding 9: validate-render-migration-report.py crashed with AttributeError when aws-design.json carried the schema-legal `ai_track: null`, demanded the deferred_shared_ai_engine text unconditionally, and rejected an honest unavailable Render baseline as COST_CURRENT_RENDER_MISSING. The AI warning and deferred-surfaces checks now run only when ai_track is deferred; a null ai_track with stale AI deferral text fails AI_DEFERRED_WARNING_UNEXPECTED. The unavailable baseline is defined as current_costs.source "unavailable" with render_monthly null; its report cell must read the literal "unavailable" (else COST_CURRENT_RENDER_UNAVAILABLE_TEXT_MISMATCH). A null amount under any other source stays COST_CURRENT_RENDER_MISSING and numeric baselines keep exact Decimal equality. Documented in estimate-infra.md. - Finding 10: parse-render-billing.py expanded every line to a month before summing, so one $30 charge split into two adjacent $15 lines doubled to 60.88. Lines are now grouped by (service_id, plan_id), each group must be contiguous (else discontiguous_period), and the group's converted amounts are normalized once over its covered interval. export.csv still yields 56.56 and the EUR mixed case 21.31. - Finding 11: parse-render-blueprint.py emitted sizeGB null when a Blueprint disk omitted it, which the normalizer rejected as disk_attachment_invalid. The parser now applies the pinned schema default of 10 GB, records sizeGB_provenance (declared or schema_default) in the disk config and service_to_disk edge, and the normalizer carries that provenance through. The exact attribute assertion in test_normalize_render_capture.py:: test_mixed_estate_normalizes_drift_lifecycle_build_sources_workflows_and_ai was extended with sizeGB_provenance "declared" to encode the corrected edge shape. Documented in schema-discover-render.md. Verification: report, billing, blueprint and normalizer script tests plus tests/test_render_fixture_contracts.py and test_render_ai_profile_contract.py pass; lint:artifacts PASS 85 artifacts, 0 errors; markdownlint clean on the touched docs.
…ough one shared alias table Addresses finding 12 of leon1418 review 5620071586 on PR aws#489. The pinned Blueprint schema accepts spellings such as `1c-2g`, `pro plus`, and `pro ultra`, and discovery preserved them verbatim, while the pricing cache and the Fargate, Elastic Beanstalk, and ElastiCache sizing tables key their rows as `1c-2gb` / `pro_plus` with no alias from the published forms. Known plans therefore missed the required exact lookup and surfaced as unavailable. - Add references/shared/render-plan-aliases.json: one table per surface (service_compute, key_value, workflow) with canonical ids, aliases, an explicit `unpriced` list, and the normalization rule (lowercase, collapse whitespace/hyphen/underscore runs to `_`, then exact lookup; unknown stays null, never a neighbouring plan). - Add scripts/render_common/plans.py with normalize_plan(surface, raw) and plan_surface(kind); unknown surfaces raise plan_surface_unknown. - parse-render-blueprint.py and normalize-render-capture.py emit config.plan_normalized next to the untouched raw plan for services, cron jobs, workers, private services, Key Value, and workflows, so Blueprint and live captures agree and Design/Estimate have a single resolution step. - Knowledge files keep legacy_aliases, point at the shared table via `alias_table`, and their rules name plan_normalized as the step before exact lookup with unresolved plans staying unavailable. - estimate-infra.md and design-infra.md describe the one alias step and the never-borrow rule. Blueprint plan enum values with no price or sizing row (free, 2c-16g, 8c-16g, 8c-64g, 12c-24g, 12c-48g, 12c-96g) are listed as unpriced so they resolve to null exactly as before. Verification: scripts/test_*.py plus tests/test_render_fixture_contracts.py 183 passed (new test_render_plans.py and test_plan_alias_table_covers_schema_enums_and_matches_knowledge_rows); lint:skill-refs PASS 1503 references; lint:artifacts PASS 85 artifacts; markdownlint 0 errors.
…dow, not the rw worker Addresses finding 1 (P1) of leon1418 review 5620071586 on PR aws#489. Discover declares `_interactive: false` / `_exec: {_agent: rw}`, but the blueprint, app-code, and billing fragments and the assembler told the dispatched worker to invoke parse-render-blueprint.py, scan-render-application.py, parse-render-billing.py, and normalize-render-capture.py. The rw worker has Read/Grep/Glob/Write/Edit only and must report WORKER_BLOCKED when shell is needed, so Blueprint-only and live-backed discovery could not complete on a host enforcing the worker contract. Apply the azure-to-aws Discover split instead of changing the tier (rwx only scopes its shell to the Terraform policy checker, and no new worker tier is introduced): - discover.md gains a "Pre-dispatch main-window actions" section: Step 0 `_init`, Step 1 attempt identity + live consent/capture (existing prose moved verbatim), Step 2 runs the three local producers into $MIGRATION_DIR/live-capture/{blueprint,application,billing}.json and lists them in the attempt manifest with digests, Step 3 runs the normalizer with all four output paths and records live-capture/normalize-result.json. The second `_preconditions` `_assert` now requires the active manifest and a status-ok normalize-result.json from the main window; fragment triggers are existence-based on manifest-listed producer outputs. - discover-blueprint.md, discover-app-code.md, discover-billing.md, discover-live.md, and discover-assemble.md are file-only readers that verify the already-written captures and normalizer outputs; the assembler cross-checks discovery_sources, inventory_fingerprint, manifest digests, and the AI-artifacts-iff-ai_profile rule, and now states both AI output paths are always passed (finding 6 wording). - SKILL.md "Discovery boundary" states every Python helper runs in the main window and the worker never invokes uv, Python, or shell. - script-abis.md adds a Caller line to all seven helper sections. - tests/test_render_phase_contracts.py adds test_discover_executable_helpers_run_only_in_main_window, which failed on the previous docs and pins the file-only contract. Verification: test_render_phase_contracts.py 11 passed; Render suite 207 passed; markdownlint 2692 files 0 errors; lint:skill-refs PASS 1508 references; tools/validate.py and tools/validate_spec.py passed.
…est and emit the selected Valkey engine Addresses findings 13 and 14 of leon1418 review 5620071586 on PR aws#489. Finding 13: promote-image.sh and the harness emitter (_promotion_script, byte-identical by contract) required an IMMUTABLE ECR repository but always tagged `deploy-<environment>`, so promoting a second digest to the same repository and environment collided with the first tag (reproduced against a local immutability-enforcing fake aws: exit 0, then 254 ImageTagAlreadyExistsException). The tag is now `deploy-<environment>-<first 12 hex of the digest>` in both copies; deployment references stay `repository@sha256:<digest>`. Finding 14: Design selects "Amazon ElastiCache for Valkey node-based" (node_type cache.m6g.large) but the fixture data.tf set `engine = "redis"` and the harness Valkey block set no engine at all (provider default redis). Both now emit `engine = "valkey"` with `engine_version = "8.0"`; the harness threads the Design mapping's aws_config.node_type into the block (design_from_inventory carries node_type for render.key_value) and assert_cache_engine_parity checks engine, engine_version, and node_type per Valkey mapping and rejects any `engine = "redis"`; assert_pack_bound_to_design calls it. Tests in tests/test_render_fixture_contracts.py: - test_image_promotion_requires_environment_and_preserves_exact_digest now expects the digest-bound tag (the old assertions encoded the colliding `deploy-production` tag). - test_two_digests_promote_to_one_repository_and_environment: two digests promote to one repository/environment against the immutability-enforcing fake; re-running the same digest fails. - test_fixture_promotion_script_matches_harness_emitter: byte parity. - test_generated_valkey_engine_matches_design_selection: fixture parity plus redis / missing engine_version / wrong node_type controls. Verification: test_render_fixture_contracts.py 21 passed including the two-run byte-identical replay (online, terraform validate exercised); terraform fmt -check and terraform init/validate passed on a scratch copy of the fixture pack.
…as offline in the Render replay Addresses finding 15 of leon1418 review 5620071586 on PR aws#489. tests/render_pipeline_harness.py::_validate_generated_pack classified any failing `terraform init` whose output contained the bare substrings `registry.terraform.io`, `timeout`, or `connection` as provider_registry_unavailable, skipped `terraform validate`, and returned passed_degraded_offline. A provider checksum-mismatch diagnostic names registry.terraform.io, so it was reproduced as "offline" success, contrary to Generate's contract that checksum, authentication, and configuration errors stay failures. Add classify_terraform_init_failure(output) and use it at the init check. It returns provider_registry_unavailable only for DNS or connectivity signatures (`dial tcp` with lookup registry.terraform.io / no such host / i/o timeout / connection refused / network is unreachable; `could not connect to registry.terraform.io`; `tls handshake timeout`; `failed to query available provider packages` only together with one of those transport phrases) and None whenever a rejected-class signature appears (checksum, does not match, doesn't match, 401, 403, unauthorized, forbidden, invalid provider, unsupported, invalid configuration, lock file, no available releases match). Rejected signatures win over connectivity signatures in mixed output. Tests in tests/test_render_fixture_contracts.py: - test_terraform_init_failure_classification_is_signature_specific (12 simulated diagnostics: DNS, connection refused, TLS timeout stay offline; checksum, 401, 403, invalid configuration, lock-file, no releases, DNS+checksum, bare substrings, empty output are failures). - test_validate_generated_pack_degrades_only_on_registry_connectivity_failure and test_validate_generated_pack_keeps_rejected_init_errors_as_failures simulate `terraform init` through harness._run on a copy of the fixture pack: the DNS case yields passed_degraded_offline without running validate; checksum, auth, config, and mixed output raise AssertionError with the original diagnostic. Verification: test_render_fixture_contracts.py 38 passed including the two-run byte-identical replay.
Contributor
Author
|
Pushed seven commits addressing all 15 inline findings from review 5620071586 (branch head Commit groups:
No follow-up integration commits were needed. Local gates run at
|
leon1418
requested changes
Oct 23, 2026
leon1418
left a comment
leon1418
left a comment
Contributor
There was a problem hiding this comment.
[🤖 AI review 🤖] Follow-up reviewed at ee740b6141010f34e7fb6ddf0dc8f3023966ab90.
Thirteen original findings are verified fixed. Two remain partially fixed: mixed direct/Azure model attribution and billing across sequential plan changes. I also verified one regression in the new mixed-manifest instructions and two issues I missed in the earlier review: Blueprint-only environment clustering and normalization of Workflow task-run details.
The bounded correction list is:
- Keep confirmed model observations attached to their actual client/source.
- Normalize sequential plan charges over the service reporting interval, or explicitly reject unsupported layouts.
- Specify null ownership digests for local producer entries even inside a live manifest.
- Keep unrelated Blueprint project environments in separate deployment clusters.
- Normalize the already-allowlisted Workflow task-run detail capture.
I will reply with fix evidence in the original conversations and resolve only those fully addressed. The new Workflow detail finding is inline; the other corrections are tracked in their existing conversations.
Validation: 740 local tests passed, including actual Terraform initialization/validation of two generated packs. The independent reviewer passed 227 unique tests after correcting a local Terraform temporary-path issue. Additional contract probes reproduce the five remaining issues; CLI/AWS fakes are not live cloud validation. Current GitHub checks are green. No source changes were made. Approval remains user-only.
| "custom_domains": "render.custom_domain", "build_sources_list": "render.build_source", | ||
| "environment_groups_list": "render.environment_group", "workflows_list": "render.workflow", | ||
| "workflow_get": "render.workflow", "workflow_versions_list": "render.workflow_version", | ||
| "workflow_tasks_list": "render.workflow_task", "workflow_task_runs_list": "render.task_run", |
Contributor
There was a problem hiding this comment.
[🤖 AI review 🤖] P2: Normalize the allowlisted Workflow task-run detail category
workflow_task_run_show is an allowed CLI category with an owned nullable_direct_object response, but CATEGORY_KINDS only maps the list category. Feeding a successful populated detail capture through the normalizer raises resource_kind_unknown and aborts discovery. I reproduced the capture-to-normalizer handoff with an explicit fake CLI and a valid ownership index; no live Render operation was used.
Map the detail category to render.task_run and retain the existing attempt/parent handling. Cover populated and nullable detail responses, while retaining ownership and digest rejection. This gap already existed at the previous reviewed head and was missed in my earlier review.
Contributor
Author
There was a problem hiding this comment.
Changed code in a9d8aa0: the workflow_task_run_show category is now mapped to render.task_run, so populated task run details normalize. Before the fix, a populated capture failed with resource_kind_unknown; after it yields a render.task_run and its render.task_attempt. The test test_populated_task_run_detail_normalizes_with_attempts_and_parent covers this.
… digest in live manifests Discover Step 2 said ownership_index_digest is null only for local-only runs, so Blueprint, application, and billing entries copied the active digest in a live manifest. normalize-render-capture.py rejects that as capture_evidence_mismatch. Align the doc and the manifest schema with the normalizer: local producer entries carry null, CLI and REST entries carry the active digest. Addresses item 1 of the PR 489 follow-up, from reviewer leon1418 (thread 4352892841, follow-up 4356086193). Adds test_live_manifest_with_local_producers_carries_null_ownership_digest.
…e called on A file with both a direct OpenAI and an AzureOpenAI client credited each source with every model in the file, recording gpt-4o-mini as confirmed Azure usage and gpt-4o as confirmed direct usage. Model identifiers now bind to the client whose receiver is on the same call line. An identifier with no resolvable receiver in a shared-client file is kept as low-confidence unattributed evidence instead of confirmed. Model lookup also matches whole tokens, so gpt-4o no longer lands on the gpt-4o-mini line. Addresses item 2 of the PR 489 follow-up, from reviewer leon1418 (thread 4352892892, follow-up 4356097561). Adds test_mixed_direct_and_azure_file_keeps_both_sources_with_models (exact per-source model sets) and test_shared_client_model_without_receiver_is_unattributed_not_confirmed.
…client A model identifier used on both the direct and Azure OpenAI clients was bound only at its first occurrence line, so the Azure call's observation was dropped and the azure_openai source was emitted with no models. Each occurrence line of a model identifier is now bound on its own, using that line's receiver. Adds a test asserting the exact model set per source when the same model is called on both clients on separate lines. Addresses item 2 of PR 489, reviewer leon1418, follow-up review.
… normalizing Billing lines were grouped by (service_id, plan_id) and each group was normalized over its own coverage. A service that moved from one plan to another mid-month was expanded to a full month per plan: $15 on starter for Sept 1-16 plus $30 on standard for Sept 16-Oct 1 produced $91.31 instead of $45.66. Group by service_id so plan segments of one service pool before the 30.4375-day normalization. Coverage is computed from merged spans, so concurrent plan segments on one service do not trip the contiguity check. Gaps in a service's coverage are still rejected as discontiguous_period. Addresses item 3 of PR aws#489, reviewer leon1418, follow-up thread 4352892903 (comment 4356099992).
leon1418
requested changes
Oct 23, 2026
leon1418
left a comment
leon1418
left a comment
Contributor
There was a problem hiding this comment.
[🤖 AI review 🤖] Follow-up reviewed at 6ccf202c171d9ed48d6d0e06785fd0ad0d0bd863 against base 9ab50636d12510dc351ff02fd7966efa4dba4341.
The 186 changed-file contents and the direct Render contracts are unchanged from the previously reviewed head. The five existing requests remain open: client/model attribution, sequential-plan billing, mixed-manifest ownership fields, Blueprint-only environment clusters, and Workflow task-run detail normalization. I reproduced them again and am not duplicating those inline comments.
I also verified three issues present in the earlier source that I missed in the previous review. They are inline below:
- A named registry-credential reference is redacted by the Blueprint parser, then rejected by normalization because its key remains.
- A report accurately using a selected
/mnt/dataEFS mount is rejected unless it also mentions the unused/var/datafixture example. Validate the actual selected mount and keep the generation/report wording consistent. - The promotion helper truncates a multiline image manifest to its first line. Preserve the complete manifest in both the fixture and replay emitter.
Validation: 740 unique parent tests and 227 independent tests passed after bounded retries for local provider-download/socket-path failures. Actual Terraform initialization/validation ran; the AWS CLI text formatter and local fakes confirmed manifest handling without an AWS API request. Current GitHub checks are green. Original failures and execution evidence are retained. No source edits, approvals, review dismissals or thread resolutions were performed.
added 4 commits
October 22, 2026 22:31
…er project Unresolved Blueprint environment names were used as bare cluster seeds, so isolated environments named the same in two projects merged into one deployment cluster. Qualify unresolved environment seeds by project. Addresses item 4 of the PR aws#489 follow-up, reviewer leon1418 (thread 4352892867, follow-up 4356093673). Adds Blueprint-only two-project cluster test and a within-project duplicate rejection test.
…captures Item 5 of PR 489 follow-up (reviewer leon1418, comment 4356084452). workflow_task_run_show is an allowed CLI category with an owned nullable_direct_object response, but CATEGORY_KINDS only mapped the list category. A populated detail capture therefore aborted with resource_kind_unknown. Map the detail category to render.task_run so the existing attempt and parent handling applies. Adds tests for a populated detail response (run, attempt, and parent edge) and a nullable detail response.
…o feature/render-to-aws-skill
…ocal Render producers Addresses item 1 of the PR 489 follow-up from reviewer leon1418. The test pins the null-digest rule for local producers (Blueprint, application, billing) in live manifests. A copied active digest is rejected by the manifest schema rule before the per-entry check, and a null digest normalizes successfully.
herosjourney
commented
Oct 23, 2026
herosjourney
left a comment
herosjourney
left a comment
Contributor
Author
There was a problem hiding this comment.
Re: follow-up review. Responses are in each open thread on head 208861a, which is a fast-forward of the reviewed head ee740b6 that also merges the main commit already on the branch.
- Manifest recipe: fixed in 1ccaf35. Local-producer entries carry a null ownership digest even in a live manifest, and the doc says so. The manifest schema rule now rejects a copied digest on a local producer with manifest_invalid, which runs before the per-entry capture_evidence_mismatch check. This is covered by the new test test_live_manifest_rejects_copied_digest_on_local_producer at 208861a. It checks that the old copied-digest recipe is rejected with manifest_invalid and that the null-digest handoff succeeds. A mutation check confirmed the test fails if the old transport rule is relaxed.
- Mixed Azure/direct attribution: fixed in 394b57f and d15737b. Each model is bound to the client it is called on, including the same model used on both clients on separate lines.
- Billing across plan changes: fixed in ae214d0. Sequential plan segments of one service are pooled before normalizing. The repro now gives 45.66 instead of 91.31, and a coverage gap raises discontiguous_period.
- Blueprint-only environment clustering: fixed in 0d7346c. Unresolved environment seeds are qualified by project, so same-named environments in two projects stay in separate clusters.
- Workflow task-run detail: fixed in a9d8aa0. workflow_task_run_show maps to render.task_run, and populated and nullable detail captures both normalize.
Gates pass at 208861a, and gitleaks finds no leaks in full history. Please re-review 208861a. I have not resolved any threads.
herosjourney
commented
Oct 23, 2026
herosjourney
left a comment
herosjourney
left a comment
Contributor
Author
There was a problem hiding this comment.
Re: follow-up review. Responses are in each open thread on head 208861a, which is a fast-forward of the reviewed head ee740b6 that also merges the main commit already on the branch.
- Manifest recipe: fixed in 1ccaf35. Local-producer entries carry a null ownership digest even in a live manifest, and the doc says so. The schema rule from that commit rejects a copied digest with manifest_invalid before the per-entry check runs. This is now covered by test_live_manifest_rejects_copied_digest_on_local_producer, added in 208861a. The test checks that the old copied-digest recipe is rejected with manifest_invalid and that the null-digest handoff succeeds.
- Mixed Azure/direct attribution: fixed in 394b57f and d15737b. Each model is bound to the client it is called on, including the same model used on both clients on separate lines.
- Billing across plan changes: fixed in ae214d0. Sequential plan segments of one service are pooled before normalizing. The repro now gives 45.66 instead of 91.31, and a coverage gap raises discontiguous_period.
- Blueprint-only environment clustering: fixed in 0d7346c. Unresolved environment seeds are qualified by project, so same-named environments in two projects stay in separate clusters.
- Workflow task-run detail: fixed in a9d8aa0. workflow_task_run_show maps to render.task_run, and populated and nullable detail captures both normalize.
Gates pass at 208861a, and gitleaks finds no leaks in full history. Please re-review 208861a. I have not resolved any threads.
leon1418
previously approved these changes
Oct 23, 2026
This PR changed two root files only to make the Render scripts' local virtual environment pass the root spec check: a .venv skip in tools/validate_spec.py and a version pin on the root lint:spec task. Both are repo-wide tooling and are outside the plugin folder. Merged Startup Advisor gates (aws#348, aws#385, aws#386, aws#387) kept their checks in the plugin's own mise.toml and changed nothing outside the folder. Restore both files to main's content. Nothing in the Render change depends on them: the Render tests pin their own dependencies in the scripts project, and no venv is committed. Verified with root validate, root build (gitleaks clean), plugin ci (575 passed) and the Render suite (236 passed) on the restored tree.
…o feature/render-to-aws-skill
6 of 40 tasks
vagmur-aws
approved these changes
Oct 27, 2026
vagmur-aws
left a comment
vagmur-aws
left a comment
There was a problem hiding this comment.
Approving. One thing still blocks merge, plus a question.
CI is red on test:telemetry. test_keys_are_exactly_this_plugin_s_skill_directories fails because render-to-aws is a skill directory but isn't in SKILL_IDS. #483 hit the same check and handled it by adding its skill to NOT_YET_MODELLED in scripts/telemetry/test/test_metric_emission.py. The same change works here: add "render-to-aws" to that set. Whichever PR merges second needs the union of both skills.
Question: should we reach out to the Blend team and put out a CR so the two models match? The plugin's SKILL_IDS/PLUGIN_SKILL_IDS have to mirror PluginSkillId in SawsStartupsAdvisor-IDEExtensionApiModel (model/types/plugin-telemetry.smithy, owned by aws-blend). That enum has neither RENDER_TO_AWS nor BEDROCK_MIGRATION_ASSESSMENT, so adding either skill to the plugin allowlist today would make every invocation fail with a 400. NOT_YET_MODELLED keeps CI green, but neither skill is counted. A CR to that package adding both values, followed by a plugin PR that moves them from NOT_YET_MODELLED into SKILL_IDS, would bring the two models back in sync.
Contributor
Author
|
Reply to #489 (review) PR 483 was closed without merging, and main has neither
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Startups hosted on Render have no supported path in
aws-startup-advisorto assess and plan a move to AWS. The plugin covers Heroku, GCP, and Azure, but a Render team today gets no inventory of their services, no AWS target recommendation, no cost estimate, and no generated migration pack. Render is a common first host for the startups this plugin targets, and its service model (Blueprints, six service types, Key Value, persistent disks, Workflows) does not map one-to-one onto any existing skill.Fix
Add a
render-to-awsskill that runs the plugin's shared Discover → Clarify → Design → Estimate → Generate → Workshop → Feedback pipeline against a Render workspace and emits the same deterministic artifact set the other migration skills produce. It uses the same phase/frontmatter DSL asazure-to-aws.What a Render user gets:
render.yamlBlueprint (validated against Render's published JSON Schema, pinned by digest), Render CLI 2.30.0 JSON captures, or consent-gated GET-only REST reads. Blueprint-only works with no Render login; CLI-only works with no Blueprint; both together are reconciled so one environment does not split into two. REST is an optional enrichment layer, never required.scan-render-application.pyfinds OpenAI, Anthropic, Google, Azure OpenAI, and OpenRouter usage and emits anai-workload-profile.jsonconforming to the shared test(aws-startup-advisor): define neutral AI workload profile #415 contract atscripts/contracts/ai-workload-profile.schema.json. OpenRouter is modeled as a gateway, never a provider. A richer Render-internal graph (workloads, call sites, relationships) is kept separately asrender-ai-discovery.json.Security posture, since discovery reads customer infrastructure:
RENDER_API_KEYis never forwarded; every retained string passes a deny-first secret classifier; raw, decompressed, depth, and wall-clock limits apply before parsing.render environment configand other resolved-variable surfaces are never invoked. Unknown config/variable endpoints are denied by default.What this PR does NOT do
ai_trackasdeferred_shared_ai_engine, keyed by the profile digest. Nothing consumes that profile yet:bedrock-migration-assessment(feat: extract Bedrock migration assessment skill #483, stage 3a of aws-startup-advisor: extract the AI/LLM engine from gcp-to-aws into its own skill so llm-to-bedrock and agent-advisor stand alone #389) still reads the legacy GCP-shaped input and no open PR teaches it the test(aws-startup-advisor): define neutral AI workload profile #415 shape. Render users will see "AI workloads detected, not yet migrated" until that lands. This PR advances no stage of aws-startup-advisor: extract the AI/LLM engine from gcp-to-aws into its own skill so llm-to-bedrock and agent-advisor stand alone #389 and closes nothing there.aws-mcpremains the plugin's sole MCP server. The skill neither configures nor invokes Render's MCP server.gcp-to-awsfor AI logic. No GCP-coupled AI code is copied or invoked.Open item for maintainers
Render emits
google+source_service: gemini_apifor direct Gemini API usage. The sharedai-capability-catalog.json(v2) lists onlyvertex_aiundergoogle. The #415 schema acceptsgemini_apiunderextensibleIdentity, so this validates, but the catalog and the scanner disagree. Either addgemini_apito the catalog or tell me to collapse it ontovertex_aiand I will update the scanner in this PR.Scope
Skills:
render-to-aws(new). The plugin now exposes 13 public skills.Files: 183 paths (166 added, 17 modified, 0 deleted), +15,690 / −44.
plugins/aws-startup-advisor/skills/render-to-aws/**: SKILL.md, phase files, selectors, knowledge tables, seven Python scripts with a locked Python 3.12 / PyYAML 6.0.2 / jsonschema 4.23.0 runtime (scripts/pyproject.toml+uv.lock), JSON Schemas, sanitized transport fixtures, expected generated pack, and 72 byte-identical vendored shared files with areferences/vendored/README.mdmanifest.plugins/aws-startup-advisor/tests/:test_render_phase_contracts.py,test_render_fixture_contracts.py,test_render_ai_profile_contract.py, andrender_pipeline_harness.py(independent two-run mixed-estate replay that generates and byte-compares the full pack).plugin.jsonand the Claude/Codex/Cursor/Devin plugin manifests, both root marketplaces, pluginREADME.md,scripts/artifact-contracts.json(fixture prefixrender-and seven Render artifact rows),scripts/validate-artifacts.py --skill render-to-aws,tools/validate_spec.py(.venvskip in symlink rejection).skills/shared/workshop/workshop-invariants.mdgains Render placeholders; the three existing vendored copies are refreshed byte-identically.mise.tomllint:specand plugintesttask pinjsonschema==4.23.0andPyYAML==6.0.2(they were unpinned onmain).Base:
mainatece684d8. This is a single commit; the branch carries none of its original base's commits (that work merged as #382).Not touched:
.mcp.json,.agents/plugins/marketplace.json,gcp-to-aws,azure-to-aws,llm-to-bedrock,agent-advisor,scripts/telemetry/sync_bundles.py(Render is not in its SKILLS list; whether it should be is a maintainer call).Validation
Local gates, all run from this branch against
mainatece684d8, all exit 0:test_render_phase_contracts.py,test_render_fixture_contracts.py,test_render_ai_profile_contract.py,skills/render-to-aws/scripts/test_*.py).mise -C plugins/aws-startup-advisor run ci: 553 passed;lint:artifacts85 artifacts / 0 errors;lint:vendored-parity72 files;lint:skill-refs1,501 refs / 0 missing; model-id, model-staleness, pricing, and consent-disclosure lints pass.mise run validate,mise run lint(2,692 Markdown files, 0 errors),mise run security(gitleaks, 709 commits, no leaks),mise run build.tools/validate.py --plugin aws-startup-advisor,tools/sync-plugin-skills.py --check,tools/validate_spec.py,tools/sync-vendored.py --check.terraform fmt,terraform init -backend=false,terraform validate, the local policy checker (POLICY_OK), report validation (REPORT_OK), schema validation, and a no-git secret scan, with evidence bound to final artifact digests.fixtures/ai-workload-profiles/*fixtures validate as positive controls; negatives prove Render never emitsopenrouteras a provider, never emitsboth/unknown, and rejectsadditionalProperties.Live validation against disposable Render fixtures informed the discovery contracts (all fixtures deleted, workspace verified empty): web, static, private, worker, cron, and Workflow services; Postgres and Key Value; projects/environments/groups; Blueprint validation; deploy failure and zero-downtime recovery; 32s free-tier cold start; 1 GB persistent disk at
/var/datawith write-through and persistence across a restart onto a different instance; service deletion cascading to disk. Workflow live coverage stopped at definition create/inventory/delete (and revealed the undocumentedwfl-ID prefix); task execution, retries, and cancellation are documented-only and the skill fails closed on them.Implementation went through 13 independent semantic-review passes before this PR. Findings those passes caught that green tests had missed include: cross-parent capture acceptance, credential-bearing repo URLs passing through REST projections, CloudFront returning
AccessDeniedfrom a private origin without a bucket policy, an ACM certificate deadlock on external DNS, and a replay harness that validated a pack it had not actually generated. All are fixed and have regression coverage.Limitations
workspaces_list,postgres_list,keyvalues_list,service_instances) are intentionally inactive and returnunsupported_contractbefore any network call because no reviewed sanitized fixture exists for them; the equivalent CLI projections are canonical.validation-report, billing, REST projections, live-capture manifest) useif/then/else, whichvalidate-artifacts.py's Draft-07 subset rejects at load; they are covered by the Render tests rather thanlint:artifactsuntil that subset grows._aborts the whole scan rather than just that model;OpenAI(base_url="https://openrouter.ai/...")yields both a gateway entry and a phantomopenai/openai_api not_observedsource;ai_trackis written to bothpreferences.jsonandaws-design.json(one producer, one digest helper).Type of Change
Checklist
python3 tools/validate.pyStartup Advisor review checklist
mise -C plugins/aws-startup-advisor run ciand all checks pass.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.