Conversation
added 3 commits
September 8, 2026 10:47
…asable specs Adds discounted-commitment analysis in two forms: a portable markdown-only skill that drives the AWS CLI, and a `commitments` Lambda MCP tool that runs the same method in Python behind the gateway. The method never presents the AWS best case as achievable. It classifies the spend profile from the observed hourly floor, sizes the commitment down accordingly, scales savings linearly, and blocks on existing under-utilization before quoting any figure — including the case the raw API will endorse, where break-even lands beyond the term. Expiry and renewal (`get_commitment_expiry`, opt-in via `regions` on the analysis tool): inventories Savings Plans and per-service reservations, derives the end date where AWS returns none (`StartTime + Duration`), buckets at 30/60/90 days, and gives a renew / renew-smaller / let-lapse / review verdict — expiry being the one moment a commitment can be resized at zero switching cost. Savings Plans are fetched once account-wide, never per region. Recommendations now carry what is actually purchasable. `RecommendationSummary` sums every specification in `details[]`, so one RDS finding can span `db.r6g.large Multi-AZ` and `db.t4g.medium Single-AZ`; a reservation only discounts usage matching its exact spec. Each finding therefore breaks down into line items naming the instance type, deployment option, engine and AZ, with size-flexible and previous-generation flags, and the family total is quoted as a budget rather than an order. Read-only throughout: no purchase, renewal, modification or cancellation API is ever called. `queries_run` counts billable Cost Explorer requests only, so the free `Describe*` inventory does not appear as spend. Reservation unit counts are never converted to money, and account-level utilization is never attributed to an individual commitment — both would need data this feature does not query. Known blind spot, declared in the report: DynamoDB reserved capacity has no describe API in any SDK. Also fixes gateway schema sync dropping the newest target. `list_gateway_targets` was called without following `nextToken`, so boto3 returned one bounded page — at 11 targets that silently omitted the 11th, and a missing target reads as "not found in gateway", failing the sync for a target that is present. Surfaced while deploying this change: `lambda-runtime` had a gateway target carrying zero tool schemas, leaving its 8 tools unreachable, and every tool added past the page boundary would have hit the same wall. Tests: 274 commitments tests, 1006 unit tests passing. Deployed and verified in dev: 34 in-place changes, all 7 new `Describe*` permissions authorized on a live invoke, and both gateway targets now carrying their full schemas.
…t paging
Closes two of the three TODOs left on the discounted-commitments PR by
replacing "verify by hand against a live account" with checks that run
offline in CI.
Field-name contract tests (tests/unit/test_commitments_contracts.py)
SpecShape and InventorySpec are lookup tables of AWS response field names,
and every read goes through .get(field). That returns None for a typo
exactly as it does for a field AWS genuinely omitted, so InstanceTpye does
not raise — it produces a recommendation with no instance type and the
report renders around the hole. Fixtures cannot catch this: a fixture
written from the same table as the code agrees with the typo and stays
green.
botocore ships the service models the SDK dispatches on, so the real names
are already on disk. These 57 tests assert every name in
RECOMMENDATION_SPECS and RESERVATION_INVENTORY is a member of the matching
output shape — containers, size fields, attribute fields, Family, Region,
the six differently-named identifier/count/type fields, and the Duration
fallback for the five families returning no explicit end date. Three
further assertions pin types a rename would quietly break: MultiAZ is
still boolean (which is why it is tested against None, not truthiness —
False means Single-AZ), commitment is still a string (AWS returns
"1.00000000", so the float() conversion is load-bearing), and
ACTIVE_SP_STATES are valid SavingsPlanState enum values (they go to the
API as a server-side filter, so an invalid one is a ValidationException in
a Lambda rather than a red test here).
Two facts are now asserted rather than assumed. DynamoDB capacity models
no SizeFlexEligible/CurrentGeneration — there is no instance, so there is
no size to flex; reading both unconditionally yields False, which is
correct, and that is pinned so nobody "fixes" the absence by inventing a
field name. And test_every_recommendation_container_is_covered compares
the modelled *InstanceDetails/*CapacityDetails containers against the ones
we know, because describe_recommendation_spec degrades to {} for an
unknown shape — a service AWS adds later would otherwise cost the report
its spec column silently.
Verified by mutation: typoing ESInstanceDetails size_fields, memorydb
id_field and the RDS MultiAZ attribute each turns exactly the expected
test red.
Gateway target paging (tests/scripts/test_sync_gateway_targets.sh)
Covers the sync.sh fix in the parent commit. boto3 does not follow
nextToken and the AWS CLI does, which is what made the bug invisible — the
CLI showed 11 targets while the same boto3 call returned 10. A dropped
target is indistinguishable from one that was never created, so the sync
reported "target not found" for a target that was present; it shipped once
and left the lambda-runtime target with zero tool schemas.
25 assertions against a boto3 stub serving scripted pages: a target on the
last page is found and updated, paging arguments are correct (no token
first, token forwarded after, maxResults=100 throughout), a single page
makes no second call, a genuinely absent target still fails loudly and
clears the hash file so the next deploy retries, schema-less tools.json
entries are skipped, and an unchanged tools.json makes no API calls at all.
Verified by reverting sync.sh to the pre-fix single call: 13 of the 25 go
red, including the exact "target not found" symptom.
Docs: new test file and count (274 -> 331) in the commitments testing
section, plus a contract-test subsection on what these do and do not
prove; the list_gateway_targets paging trap added to the development.md
gotchas beside its tools/list sibling.
Not closed: the third TODO still needs an account with OpenSearch or
DynamoDB steady-state usage. The dev account holds no reservations and no
recommendations, so populated-field verification for those two shapes
remains outstanding. The contract tests narrow it to "AWS omitted it"
rather than "we spelled it wrong".
Neither service is in scope for this deployment, so there is no live usage to size a reservation against and no live assertion to make. Reframes the contract-test limitation from an outstanding verification step to a stated boundary: those two shapes rest on the contract tests and unit fixtures, which already guarantee a blank spec column means AWS omitted the field rather than that the field name is wrong.
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
1 participant
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.
Summary
Adds a discounted commitments capability to the platform: a read-only
commitmentsMCP tool plus a markdown-only skill that turns AWS Cost Explorer / Cost Optimization Hub data into a Savings Plan / Reserved Instance recommendation a human can actually act on.Three things it does that the raw AWS recommendations do not:
renew/renew-smaller/let-lapse/review.RecommendationSummarysums across everydetails[]entry, so one Finding can spandb.r6g.large Multi-AZanddb.t4g.medium Single-AZ. A reservation only discounts usage matching its exact spec, so the family total is a budget, not an order. Each recommendation now carries a per-LineItembreakdown with the instance type, AZ/deployment type, engine and platform needed to place the purchase.Components
src/lambda/mcp/commitments/— 5 modules (api,collect,analyze,report, handler), 8 tools registered intools.jsonskills/discounted-commitments/— 3 markdown files, zero code (portability by reusing existing tooling, no bundled scripts)cost-operationsagent + supervisor routing inhierarchy.jsondiscounted_commitments.jsonfor both the agent and core-api surfacesdocs/skills/discounted-commitments.md(14 §), plus README / cost-operations / development updatestests/unit/Incidental fix
scripts/lib/sync.shlisted gateway targets without followingnextToken. boto3 does not auto-paginate (the AWS CLI does), so once the gateway held more than one page of targets the newest ones silently vanished from the list and read as "not found", failing the schema sync. This had already left thelambda-runtimetarget from #23 with zero tool schemas registered, making its 8 tools unreachable. Now paginated; re-deployed and confirmed all 8 schemas present.Safety posture
Read-only by construction. IAM grants only
Get*/List*/Describe*(17 actions). The tool never callsCreateSavingsPlan,PurchaseReservedInstances*,StartCommitmentPurchaseAnalysis,ModifyReservedInstances,ReturnSavingsPlanorDeleteQueuedSavingsPlan— it recommends, a human purchases.Reporting constraints enforced in code and documented: never present the AWS best case as achievable; never fabricate figures; never convert reservation unit counts to money; never imply per-commitment utilization; never recommend a reservation without its specification; never present a multi-specification total as a single purchase; lead with blockers.
Test plan
make test-unit— 1076 unit + topology tests pass (331 in the commitments suite). Two pre-existing collection errors intest_agui_*.pyare unrelated:ag-ui-strandsis a container-only dep (src/agents/frontend/requirements.txt) that is not installed in the dev venv, and those errors predate this branch.make test-scripts— 83 assertions across 4 shell suites pass, including the new gateway-paging suite.tests/unit/test_commitments_contracts.py, 57 tests).SpecShape/InventorySpecare tables of field names read via.get(field), so a typo returnsNoneexactly like a field AWS omitted — it never raises, it just produces a recommendation with no instance type. Fixtures can't catch that; a fixture written from the same table agrees with the typo. These tests assert membership in the real output shapes, and additionally pin thatMultiAZis stillboolean,commitmentis still astring, andACTIVE_SP_STATESare validSavingsPlanStateenum values. Verified by mutation — three deliberate typos each turned exactly the expected test red.tests/scripts/test_sync_gateway_targets.sh, 25 assertions against a boto3 stub serving scripted pages): a target on the last page is found and updated, paging args are correct, a genuinely absent target still fails loudly and clears the hash file, and an unchangedtools.jsonmakes no API calls. Verified by revertingsync.shto the pre-fix single call — 13 of 25 go red, including the exacttarget not foundsymptom.dev(make deploy-auto) — 0 add / 34 change / 0 destroyget_commitment_expiryinvoke returnsdata_source: livewith no collection warningsReservation specifications for OpenSearch (
ESInstanceDetails) and DynamoDB(
ReservedCapacityDetails) are covered by the contract tests and unit fixtures only —neither service is in scope for this deployment, so there is no live usage to size a
reservation against. The contract tests are the guarantee that matters here: a blank
spec column can only mean AWS omitted the field, not that the field name is wrong.