From 892b1d7e07068698e2d3daed7612acb6fe0ccb0a Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sun, 6 Sep 2026 05:34:41 +0900 Subject: [PATCH 1/8] test: add reproducer for the four unreachable Lex services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RED: CGO_ENABLED=0 go test ./internal/gateway/ -run TestDetectProtocol_Lex --- FAIL: TestDetectProtocol_Lex/models_v1_get_bots --- FAIL: TestDetectProtocol_Lex/models_v1_get_intents --- FAIL: TestDetectProtocol_Lex/models_v1_builtin_intents --- FAIL: TestDetectProtocol_Lex/models_v2_list_bots --- FAIL: TestDetectProtocol_Lex/models_v2_create_bot --- FAIL: TestDetectProtocol_Lex/models_v2_describe_bot --- FAIL: TestDetectProtocol_Lex/runtime_v1_get_session --- FAIL: TestDetectProtocol_Lex/runtime_v1_put_session --- FAIL: TestDetectProtocol_Lex/runtime_v2_get_session --- FAIL: TestDetectProtocol_Lex/runtime_v2_delete_session --- PASS: TestDetectProtocol_Lex/delete_bot_is_contested Every failure reads `expected: , actual: "lex"` — the request never leaves the contested alias. The one PASS is the genuine collision (DELETE /bots/{id}, claimed by both model services), which must stay refused. Recorded in .claude/tdd/aws-service-coverage-100-milestone-6.tdd.md as "Coverage and known gaps", item 3. --- internal/gateway/protocol_test.go | 55 +++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/internal/gateway/protocol_test.go b/internal/gateway/protocol_test.go index f74b8255..6be8c2f1 100644 --- a/internal/gateway/protocol_test.go +++ b/internal/gateway/protocol_test.go @@ -8,6 +8,12 @@ import ( "testing" "github.com/stretchr/testify/assert" + + // The route tables the shared-signing-name split reads are registered by + // this package's init, which only runs for an importer. cmd/devcloud has + // its own blank import; without one here the tables are empty and every + // route-based routing decision silently answers "no candidate models it". + _ "github.com/skyoo2003/devcloud/internal/generated/crudregistry" ) func TestDetectProtocol_RESTXML(t *testing.T) { @@ -65,6 +71,55 @@ func TestDetectProtocol_S3Control(t *testing.T) { } } +// TestDetectProtocol_Lex covers the four services that share a signing name no +// service is called. All four boto3 Lex clients sign as "lex", and codegen +// leaves the alias contested rather than sending three services' traffic to the +// fourth — so every Lex call died as UnknownService, which docs/coverage.md +// published as an exclusion. +// +// The split needs no new routing: resolveSharedSigningName already picks the +// sibling whose route table models the request. It was never reached, because +// signingNameOf only recognised a *member* service ID, and "lex" is the group's +// key. +// +// The route tables are the real ones (crudregistry is blank-imported by this +// package's tests), so this asserts what a boto3 caller actually gets rather +// than what a hand-built table would. +func TestDetectProtocol_Lex(t *testing.T) { + cases := []struct{ name, method, path, want string }{ + // lex-models: GetBots. lexv2-models claims /bots too, but under POST and PUT. + {"models_v1_get_bots", "GET", "/bots", "lexmodelbuildingservice"}, + {"models_v1_get_intents", "GET", "/intents", "lexmodelbuildingservice"}, + {"models_v1_builtin_intents", "GET", "/builtins/intents", "lexmodelbuildingservice"}, + // lexv2-models: ListBots is POST /bots, CreateBot is PUT /bots. + {"models_v2_list_bots", "POST", "/bots", "lexmodelsv2"}, + {"models_v2_create_bot", "PUT", "/bots", "lexmodelsv2"}, + {"models_v2_describe_bot", "GET", "/bots/b1", "lexmodelsv2"}, + // lex-runtime addresses sessions under /bot/ singular. + {"runtime_v1_get_session", "GET", "/bot/b/alias/a/user/u/session", "lexruntimeservice"}, + {"runtime_v1_put_session", "POST", "/bot/b/alias/a/user/u/session", "lexruntimeservice"}, + // lexv2-runtime spells it botAliases; lexv2-models spells it botaliases. + {"runtime_v2_get_session", "GET", "/bots/b/botAliases/a/botLocales/l/sessions/s", "lexruntimev2"}, + {"runtime_v2_delete_session", "DELETE", "/bots/b/botAliases/a/botLocales/l/sessions/s", "lexruntimev2"}, + // The one genuine collision, asserted rather than left to be found: + // lex-models' DeleteBot is DELETE /bots/{name} and lexv2-models' is + // DELETE /bots/{botId}. Two siblings model it, so the request stays + // where it was — an honest UnknownService rather than a coin flip that + // deletes the wrong bot. + {"delete_bot_is_contested", "DELETE", "/bots/b1", "lex"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + req := httptest.NewRequest(c.method, c.path, nil) + req.Header.Set("Authorization", + "AWS4-HMAC-SHA256 Credential=AKIA/20130524/us-east-1/lex/aws4_request, Signature=abc") + proto, service := DetectProtocol(req) + assert.Equal(t, "rest-json", proto) + assert.Equal(t, c.want, service) + }) + } +} + func TestDetectProtocol_JSON10_DynamoDB(t *testing.T) { req := httptest.NewRequest("POST", "/", strings.NewReader(`{}`)) req.Header.Set("X-Amz-Target", "DynamoDB_20120810.PutItem") From a04736e075c2e5fea7aebeb7a69e67b07ff96ebb Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sun, 6 Sep 2026 05:38:39 +0900 Subject: [PATCH 2/8] fix: the four Lex services were reachable by no boto3 caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GREEN: CGO_ENABLED=0 go test ./internal/gateway/ -run TestDetectProtocol_Lex → ok CGO_ENABLED=0 go test ./... → all pass DEVCLOUD_BIN=dist/devcloud pytest tests/compatibility/ → 1123 passed, 17 skipped, 2 xfailed (was 1119 passed) All four Lex clients sign as "lex" and no service is called "lex", so the alias stays contested. normalizeServiceID hands the unresolved name through, the registry has nothing under it, and every Lex call died as UnknownService — four services registered, counted in "serving >=1 operation", and reachable by nobody. The fix needed no new routing. resolveSharedSigningName already picks the sibling whose route table models the request; it was never reached, because signingNameOf only recognised a *member* service ID and "lex" is the group's key. One map lookup. The four route tables separate cleanly on method and literal segment: GET /bots lexmodelbuildingservice POST /bots lexmodelsv2 (ListBots) PUT /bots lexmodelsv2 (CreateBot) GET /bot/{n}/alias/{a}/user/{u}/session lexruntimeservice GET /bots/{b}/botAliases/.../sessions/{s} lexruntimev2 One genuine collision, DeleteBot at DELETE /bots/{id}, claimed by both model services. It stays refused: deleting the wrong bot is worse than an honest UnknownService. Asserted, not left to be found. The pin test told whoever fixed this to delete it, so it is deleted and replaced by a check on the direction that still matters — UNREACHABLE_FROM_BOTO3 must stay empty. docs/coverage.md's compatibility-tested figure moves 199 -> 203, the Lex exclusion section is rewritten, and the stale claim that each Lex service is "reached by its own unambiguous name" is corrected: no boto3 caller has one. Plan: .claude/plans/aws-service-coverage-100-milestone-7.plan.md, Task 1 --- changes/unreleased/Fixed-20260906-153000.yaml | 5 ++ docs/coverage.md | 51 +++++++++++-------- internal/gateway/protocol.go | 11 ++++ tests/compatibility/_coverage.py | 30 ++++------- tests/compatibility/test_service_smoke.py | 47 +++++++---------- 5 files changed, 73 insertions(+), 71 deletions(-) create mode 100644 changes/unreleased/Fixed-20260906-153000.yaml diff --git a/changes/unreleased/Fixed-20260906-153000.yaml b/changes/unreleased/Fixed-20260906-153000.yaml new file mode 100644 index 00000000..76448396 --- /dev/null +++ b/changes/unreleased/Fixed-20260906-153000.yaml @@ -0,0 +1,5 @@ +kind: Fixed +body: 'The four Amazon Lex services are reachable from boto3. `lex-models`, `lex-runtime`, `lexv2-models` and `lexv2-runtime` all sign as `lex`, and no service is called `lex`, so the alias stayed contested and every Lex call died as `UnknownService` — the services were registered, counted as serving operations, and reachable by nobody. The shared-signing-name split that already separates `mediastore-data`, `sagemaker-runtime` and Timestream now recognises a signing name that is a group key rather than a member service ID, so each request is answered by the sibling whose route table models its method and path. `DeleteBot` is claimed by two of them at `DELETE /bots/{id}` and is still refused rather than guessed at. Compatibility-tested services: 199 to 203' +time: 2026-09-06T15:30:00.000000+09:00 +custom: + Issue: "146" diff --git a/docs/coverage.md b/docs/coverage.md index d6754e9e..17a0863f 100644 --- a/docs/coverage.md +++ b/docs/coverage.md @@ -12,7 +12,7 @@ things: | **Registered** | The gateway routes the service. A call reaches DevCloud instead of falling through to real AWS. | **205** | | **Serving ≥1 operation** | At least one operation returns a real, store-backed answer — hand-written or served by the generic CRUD engine. | **201** | | **Registered-only** | Routed, but every operation declines with a clean AWS error. Nothing is served. | **4** | -| **Compatibility-tested** | A boto3 test exercises the service in CI and passes — either serving an operation or declining cleanly. | **199** | +| **Compatibility-tested** | A boto3 test exercises the service in CI and passes — either serving an operation or declining cleanly. | **203** | Every number on this page is asserted against the binary by `go test ./cmd/devcloud/`. Editing one here without the code moving fails CI, and @@ -63,13 +63,13 @@ and `sagemakerruntimehttp2` (`InvokeEndpoint*`), and `rds-data` (`ExecuteStatement`, `BeginTransaction`, `CommitTransaction`, `RollbackTransaction`). No protocol change reaches them. -## Why the compatibility-tested number is 199, not 205 +## Why the compatibility-tested number is 203, not 205 Every registered service is exercised by `tests/compatibility/test_service_smoke.py`, which parametrises over the generated service list rather than a hand-written one — so a service cannot be registered and quietly go untested, which is what 31 of them were until this was -measured. Six are excluded, in two groups, and neither group is a backlog. +measured. Two are excluded, and they are not a backlog. **Two have no boto3 client at all.** botocore publishes 431 clients and neither `sagemakerruntimehttp2` nor `transcribestreaming` is among them — @@ -77,22 +77,25 @@ measured. Six are excluded, in two groups, and neither group is a backlog. No boto3 test can exist for a client that does not exist. This is a property of the AWS SDK, not of DevCloud, and it is the ceiling on this number. -**Four are registered and unreachable.** All four Lex clients — `lex-models`, -`lex-runtime`, `lexv2-models`, `lexv2-runtime` — sign as `lex`, and no service -is named `lex`, so the alias stays contested and routes nowhere rather than -sending one service's traffic to another. The consequence, which this page did -not state until it was measured: the "reached by its own unambiguous name" -escape below does not exist for a boto3 caller, because boto3 signs with the -contested name and offers nothing else. The four are counted in *serving ≥1 -operation*, because their operations are classified and would be served; they -are not counted here, because nothing can ask for them. - -Splitting them on the URL is what resolved `opensearch` / `elasticsearchservice` -and API Gateway v1 / v2, and it does not work here: `/bots` is the first path -segment for three of the four. A correct split needs full path matching across -all four, which is routing work rather than coverage work. -`test_lex_services_are_unreachable_from_boto3` pins the current state, so -fixing the routing fails that test and moves this number up with it. +**The four Lex services used to be a third exclusion, and are not any more.** +All four Lex clients — `lex-models`, `lex-runtime`, `lexv2-models`, +`lexv2-runtime` — sign as `lex`, and no service is named `lex`, so the alias is +contested and still routes nowhere on its own. What this page got wrong was the +escape it offered: "reached by its own unambiguous name" does not exist for a +boto3 caller, because boto3 signs with the contested name and offers nothing +else. Measuring it found four services registered, counted as serving, and +reachable by nobody. + +The split that resolved `opensearch` / `elasticsearchservice` and API Gateway +v1 / v2 does work here, and it needed no new routing. Services that share a +signing name are already separated by asking which one's route table models the +request; the group was simply never reached, because the lookup only recognised +a *member* service ID and `lex` is the group's *key*. Each Lex request now +reaches the sibling that models its method and path — `GET /bots` is +`lex-models`, `POST /bots` is `lexv2-models`, `/bot/…/session` is `lex-runtime`, +`/bots/…/botAliases/…/sessions/…` is `lexv2-runtime`. One operation is claimed by +two of them, `DeleteBot` at `DELETE /bots/{id}`, and it is still refused rather +than guessed at: deleting the wrong bot is worse than an honest error. Being engine-servable is not the same as being served. Thirteen of the fifteen `query` services and three of the four `rest-xml` ones have hand-written @@ -182,8 +185,14 @@ so `sagemaker` is claimed by eight services at once. Where exactly one claimant carries the name as its own service ID, that service wins and the rest are treated as borrowers — see `codegen.BuildAliases`. Where none does, the alias is left unrouted rather than guessed: all four Lex services sign as `lex` and none -is called `lex`, so `lex` routes nowhere and each Lex service is reached by its -own unambiguous name. +is called `lex`, so `lex` resolves to nothing on its own. + +An unrouted alias is not the same as an unreachable service, and this page once +conflated them — it claimed each Lex service was "reached by its own unambiguous +name", which no boto3 caller has, since boto3 sends the contested name and +nothing else. The name is still not guessed; the request is instead handed to +the group of services that share it, and answered by the one whose route table +models its method and path. Two candidates, or none, and it is refused. ## The target, and the rule that set it diff --git a/internal/gateway/protocol.go b/internal/gateway/protocol.go index 3336f788..679705e8 100644 --- a/internal/gateway/protocol.go +++ b/internal/gateway/protocol.go @@ -318,7 +318,18 @@ func resolveSharedSigningName(service string, match func(candidate string) bool) // signingNameOf finds the signing name whose group contains service. The groups // are small and few, and this runs once per request that reaches a shared name. +// +// A service may also arrive *as* a signing name. Most shared names are the +// parent service's own ID, so the group is found by membership — but four Lex +// services sign as "lex" and none of them is called that, so the alias stays +// contested and normalizeServiceID hands the unresolved name straight through. +// Recognising the group key is what lets those requests reach the split at all; +// the split then answers with the sibling that models the request, or refuses +// when two do. func signingNameOf(service string) string { + if _, ok := aliases.SigningSiblings[service]; ok { + return service + } for name, group := range aliases.SigningSiblings { for _, id := range group { if id == service { diff --git a/tests/compatibility/_coverage.py b/tests/compatibility/_coverage.py index 8e3dc563..7a025406 100644 --- a/tests/compatibility/_coverage.py +++ b/tests/compatibility/_coverage.py @@ -63,27 +63,17 @@ # Registered, counted as serving operations, and reachable by no boto3 caller. # -# All four Lex clients sign as "lex" and none of the four services is called -# "lex", so codegen.BuildAliases leaves the name contested and unrouted — by -# design, because guessing would send one service's traffic to another. The -# consequence docs/coverage.md did not state until this was measured: the -# "reached by its own unambiguous name" escape does not exist for a boto3 -# caller, because boto3 signs with the contested name and nothing else. +# Empty, and kept as the name of a category rather than deleted: a service falls +# into it again the next time two services share a name nobody is called. # -# Splitting them on the URL is what resolved opensearch/elasticsearchservice and -# apigateway v1/v2, and it does not work here: /bots is the first segment for -# lex-model-building-service, lex-models-v2 AND lex-runtime-v2. A correct split -# needs full path matching across four services, which is routing work rather -# than coverage work. -# -# Pinned by test_lex_services_are_unreachable_from_boto3 so that fixing the -# routing fails this test and forces the published figure up with it. -UNREACHABLE_FROM_BOTO3 = { - "lexmodelbuildingservice": "signs as the contested alias 'lex'", - "lexmodelsv2": "signs as the contested alias 'lex'", - "lexruntimeservice": "signs as the contested alias 'lex'", - "lexruntimev2": "signs as the contested alias 'lex'", -} +# The four Lex services were the whole membership. They still sign as "lex", no +# service is still called "lex", and the alias is still contested and unrouted — +# what changed is that the shared-signing-name split now recognises "lex" as a +# group *key*, not only as a member service ID, so each request is answered by +# the sibling whose route table models its method and path. The one operation +# two siblings both model (DELETE /bots/{id}) is still refused rather than +# guessed at. +UNREACHABLE_FROM_BOTO3: dict[str, str] = {} # A resource that does not exist in an empty store is a served answer: the engine # looked in the store and reported honestly. diff --git a/tests/compatibility/test_service_smoke.py b/tests/compatibility/test_service_smoke.py index a7cb14d1..248c69e9 100644 --- a/tests/compatibility/test_service_smoke.py +++ b/tests/compatibility/test_service_smoke.py @@ -104,37 +104,24 @@ def test_services_without_a_boto3_client_are_exactly_the_known_two(): ) -@pytest.mark.parametrize("service", sorted(_coverage.UNREACHABLE_FROM_BOTO3)) -def test_lex_services_are_unreachable_from_boto3(service_client, service): - """The four Lex services are registered, counted as served, and unroutable. - - All four boto3 Lex clients sign as "lex", no service is named "lex", so the - alias stays contested and unrouted rather than guessed. docs/coverage.md - said each Lex service is reachable "by its own unambiguous name"; a boto3 - caller has no such name to use, which is what this pins. - - It asserts UnknownService deliberately. Making Lex routable is routing work, - not coverage work, and when it lands this test fails — which is the signal to - move these four into the parametrization above and raise the published - compatibility-tested figure with them. +def test_no_registered_service_is_unreachable_from_boto3(): + """The exclusion category stays empty, or the published figure moves with it. + + This replaces test_lex_services_are_unreachable_from_boto3, which pinned the + four Lex services as unroutable and told whoever fixed the routing to delete + it. The routing was fixed — "lex" is now recognised as a signing-name group + key, so each Lex request reaches the sibling that models its path — and the + four joined the floor parametrization below. + + What survives is the direction of the check rather than the pin: a service + that becomes unreachable has to be named in UNREACHABLE_FROM_BOTO3, and + naming it lowers the compatibility-tested figure gated above. """ - entry = MANIFEST[service] - name = _coverage.boto3_name(service) - client = service_client(name) - - candidates = _coverage.probe_operations(name, entry["servedOps"]) - assert candidates, f"{service} has no served operation to probe" - operation, needs_input = candidates[0] - params = _coverage.stub_params(name, operation) if needs_input else {} - - with pytest.raises(ClientError) as excinfo: - _call(client, operation, params) - - code = excinfo.value.response.get("Error", {}).get("Code") - assert code == "UnknownService", ( - f"{service} ({name}.{operation}) answered {code}, not UnknownService. " - "If Lex routing was fixed, move these services into " - "test_registered_service_meets_the_floor and update docs/coverage.md." + assert _coverage.UNREACHABLE_FROM_BOTO3 == {}, ( + "a registered service is unreachable from boto3: " + f"{sorted(_coverage.UNREACHABLE_FROM_BOTO3)}. It drops out of the " + "compatibility-tested figure in docs/coverage.md, which is gated by " + "test_published_compatibility_tested_figure_matches_this_suite." ) From ae83be2b3f41bd0efe53546070c49963a6993cd6 Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sun, 6 Sep 2026 05:40:59 +0900 Subject: [PATCH 3/8] test: add reproducer for hand-verified operations no route reaches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RED: DEVCLOUD_BIN=dist/devcloud pytest tests/compatibility/test_handverified_is_reachable.py FAILED [appsync.ListApis] FAILED [eks.ListAccessPolicies] FAILED [opensearch.ListApplications] 3 failed in 1.56s Each answers NotImplemented. All three are labelled hand-verified in the fidelity manifest, and all three are implemented — codegen reads the case clause, and the case clause is there. What is missing is the route: the gateway passes no operation name for rest-json, so the provider recovers it from method and path with a hand-written resolver, and none of these three resolvers knows the path its own model publishes. appsync/ListApis GET /v2/apis resolveOp strips only /v1 eks/ListAccessPolicies GET /access-policies no case at all opensearch/ListApplications GET /2021-01-01/opensearch/list-applications resolveOp knows only GET /application Recorded in .claude/tdd/aws-service-coverage-100-milestone-6.tdd.md as "Coverage and known gaps", item 2. --- .../test_handverified_is_reachable.py | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 tests/compatibility/test_handverified_is_reachable.py diff --git a/tests/compatibility/test_handverified_is_reachable.py b/tests/compatibility/test_handverified_is_reachable.py new file mode 100644 index 00000000..3d949bf9 --- /dev/null +++ b/tests/compatibility/test_handverified_is_reachable.py @@ -0,0 +1,78 @@ +"""An operation the manifest calls hand-verified must be reachable. + +`hand-verified` is the strongest label DevCloud publishes: someone wrote code for +this operation. Codegen derives it by reading the `case` clauses in a provider's +dispatch switch — which proves the code exists, not that any request arrives at +it. + +For a REST-JSON provider those are two different questions. The gateway passes no +operation name (there is no `X-Amz-Target` to read), so the provider recovers it +from the method and path with a hand-written resolver. Where that resolver does +not know a path, the operation is unreachable however carefully it was +implemented, and the manifest says `hand-verified` about code nothing can call. + +Three were found this way by Milestone 6's per-service floor test, which had +tolerated them because it needs only *one* served operation per service to pass. +They are asserted individually here, because that is the only granularity at +which the defect is visible. +""" + +import pytest +from botocore import xform_name +from botocore.exceptions import ClientError + +import _coverage + +MANIFEST = _coverage.load_manifest() + +# service id, boto3 client name, operation, and the route boto3 actually sends. +# The route is quoted from the service's own model (the generated CRUD registry +# holds the same URI), because it is what the provider's resolver has to know. +CASES = [ + ("appsync", "appsync", "ListApis", "GET /v2/apis"), + ("eks", "eks", "ListAccessPolicies", "GET /access-policies"), + ( + "opensearch", + "opensearch", + "ListApplications", + "GET /2021-01-01/opensearch/list-applications", + ), +] + + +@pytest.mark.parametrize( + "service,client_name,operation,route", + CASES, + ids=[f"{s}.{op}" for s, _, op, _ in CASES], +) +def test_handverified_operation_is_reachable( + service_client, service, client_name, operation, route +): + """The operation answers, rather than reporting itself unimplemented. + + What is asserted is deliberately weak: any answer other than an error. The + manifest claims the operation is implemented, so the failure being caught is + the provider not finding its own implementation — not the shape of what it + returns. + """ + assert operation in MANIFEST[service]["servedOps"], ( + f"{service}.{operation} is no longer listed as served, so this test " + "asserts a claim the manifest is not making. Delete the case, or fix " + "whatever stopped the manifest from making it." + ) + + client = service_client(client_name) + params = _coverage.stub_params(client_name, operation) + + try: + response = getattr(client, xform_name(operation))(**params) + except ClientError as exc: + code = exc.response.get("Error", {}).get("Code") + pytest.fail( + f"{service}.{operation} ({route}) answered {code}. The manifest calls " + "it hand-verified, so a provider case clause implements it — but the " + "provider's own path resolver never reaches that clause, so the " + "operation arrives empty and the dispatch falls through to default." + ) + + assert response["ResponseMetadata"]["HTTPStatusCode"] == 200 From fd2510a5e6ba616d5504303efc8b22305f8f8154 Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sun, 6 Sep 2026 05:44:02 +0900 Subject: [PATCH 4/8] fix: three hand-verified operations were reached by no route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GREEN: DEVCLOUD_BIN=dist/devcloud pytest tests/compatibility/test_handverified_is_reachable.py → 3 passed CGO_ENABLED=0 go test ./... → all pass DEVCLOUD_BIN=dist/devcloud pytest tests/compatibility/ → 1126 passed, 17 skipped, 2 xfailed (was 1123) make codegen; git status --porcelain internal/generated → clean golangci-lint run → 0 issues appsync/ListApis, eks/ListAccessPolicies and opensearch/ListApplications are each implemented by a case clause the provider already has. Codegen reads that clause and labels the operation hand-verified — which proves the code exists, not that any request arrives at it. For rest-json those are different questions: the gateway passes no operation name, so the provider recovers it from method and path with a hand-written resolver, and none of these three resolvers knew the route its own model publishes. The resolvers are partial copies of a table codegen already emits. Each provider now ends with crud.Route(service, method, uri) instead of losing the request. The fallback fires only where the resolver produced nothing, so it cannot displace a decision a provider made — it fills gaps. crud.Route is HasRoute's implementation, exported; HasRoute becomes its one-line caller. Nothing else in the gateway changes. No published figure moves. These operations were already counted as served; what changes is that the count is now true about them. 27 other providers hand-roll the same kind of resolver. Whether any of them has the same gap is not measured here — the compat manifest carries served operations but not their tier, so the fleet-wide version of this test needs a codegen change first. Recorded, not guessed at. Plan: .claude/plans/aws-service-coverage-100-milestone-7.plan.md, Task 2 --- changes/unreleased/Fixed-20260906-160000.yaml | 5 ++++ internal/services/appsync/provider.go | 9 ++++++ internal/services/eks/provider.go | 9 ++++++ internal/services/opensearch/provider.go | 10 +++++++ internal/shared/crud/crud.go | 28 +++++++++++++++---- 5 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 changes/unreleased/Fixed-20260906-160000.yaml diff --git a/changes/unreleased/Fixed-20260906-160000.yaml b/changes/unreleased/Fixed-20260906-160000.yaml new file mode 100644 index 00000000..9dc73a9c --- /dev/null +++ b/changes/unreleased/Fixed-20260906-160000.yaml @@ -0,0 +1,5 @@ +kind: Fixed +body: 'Three operations the fidelity manifest calls `hand-verified` are now reachable. `appsync.ListApis`, `eks.ListAccessPolicies` and `opensearch.ListApplications` were each implemented by a `case` clause in their provider, but the provider''s hand-written path resolver did not know the route boto3 sends — `GET /v2/apis`, `GET /access-policies` and `GET /2021-01-01/opensearch/list-applications` — so the operation arrived empty and the call answered `NotImplemented`. Each provider now falls back to its own model''s route table when its resolver recognises nothing, which recovers the operation instead of losing it. No published figure moves: these operations were already labelled served, and the label is now true' +time: 2026-09-06T16:00:00.000000+09:00 +custom: + Issue: "147" diff --git a/internal/services/appsync/provider.go b/internal/services/appsync/provider.go index d5b3114f..6195e43b 100644 --- a/internal/services/appsync/provider.go +++ b/internal/services/appsync/provider.go @@ -15,6 +15,7 @@ import ( "github.com/skyoo2003/devcloud/internal/plugin" "github.com/skyoo2003/devcloud/internal/shared" + "github.com/skyoo2003/devcloud/internal/shared/crud" ) // Provider implements the DeepdishControlPlaneService (AppSync) service. @@ -62,6 +63,14 @@ func (p *Provider) HandleRequest(_ context.Context, op string, req *http.Request if op == "" { op = resolveOp(req.Method, path) } + // resolveOp is hand-written and knows the paths somebody thought to add. The + // model's own route table knows all of them, so a path it does not recognise + // is recovered rather than lost: ListApis is GET /v2/apis and resolveOp only + // strips /v1, which left the case clause below unreachable while the + // fidelity manifest called it hand-verified. + if op == "" { + op = crud.Route("appsync", req.Method, req.URL.RequestURI()) + } switch op { // GraphqlApi diff --git a/internal/services/eks/provider.go b/internal/services/eks/provider.go index 6688b85a..0f00a7b6 100644 --- a/internal/services/eks/provider.go +++ b/internal/services/eks/provider.go @@ -14,6 +14,7 @@ import ( "github.com/skyoo2003/devcloud/internal/plugin" "github.com/skyoo2003/devcloud/internal/shared" + "github.com/skyoo2003/devcloud/internal/shared/crud" "github.com/skyoo2003/devcloud/internal/storage/sqlite" ) @@ -62,6 +63,14 @@ func (p *Provider) HandleRequest(_ context.Context, op string, req *http.Request if op == "" { op = resolveOp(req.Method, path) } + // resolveOp is hand-written and knows the paths somebody thought to add. The + // model's own route table knows all of them, so a path it does not recognise + // is recovered rather than lost: ListAccessPolicies is GET /access-policies + // and resolveOp has no case for it, which left the case clause below + // unreachable while the fidelity manifest called it hand-verified. + if op == "" { + op = crud.Route("eks", req.Method, req.URL.RequestURI()) + } switch op { // ── Cluster ────────────────────────────────────────────────────────────── diff --git a/internal/services/opensearch/provider.go b/internal/services/opensearch/provider.go index f9fda236..cb57dd52 100644 --- a/internal/services/opensearch/provider.go +++ b/internal/services/opensearch/provider.go @@ -14,6 +14,7 @@ import ( "github.com/skyoo2003/devcloud/internal/plugin" "github.com/skyoo2003/devcloud/internal/shared" + "github.com/skyoo2003/devcloud/internal/shared/crud" ) // Provider implements the OpenSearchService service. @@ -46,6 +47,15 @@ func (p *Provider) HandleRequest(_ context.Context, op string, req *http.Request if op == "" { op = resolveOp(req.Method, req.URL.Path) } + // resolveOp is hand-written and knows the paths somebody thought to add. The + // model's own route table knows all of them, so a path it does not recognise + // is recovered rather than lost: ListApplications is + // GET /2021-01-01/opensearch/list-applications and resolveOp knows only + // GET /application, which left the case clause below unreachable while the + // fidelity manifest called it hand-verified. + if op == "" { + op = crud.Route("opensearch", req.Method, req.URL.RequestURI()) + } body, err := io.ReadAll(req.Body) if err != nil { diff --git a/internal/shared/crud/crud.go b/internal/shared/crud/crud.go index beaca3f3..82b1bcd1 100644 --- a/internal/shared/crud/crud.go +++ b/internal/shared/crud/crud.go @@ -105,16 +105,34 @@ func Register(service string, ops map[string]OpMeta) { routes[service] = rs } +// Route returns the operation a service models at this method and URI, or "" if +// it models none. +// +// The table is the service's own model, so this is the answer a hand-written +// path resolver inside a provider is trying to reproduce. A provider whose +// resolver does not recognise a path can end with this instead of "": the +// operation is recovered from the model rather than lost, and a `case` clause +// the provider already has stops being unreachable. +// +// It answers "" rather than an error for a service the engine holds no routes +// for. Registration happens in the generated crudregistry's init, which only +// runs for an importer — a provider unit test that does not import it sees an +// empty table and falls back on the provider's own resolver, which is the +// behaviour it had before. +func Route(service, method, uri string) string { + mu.RLock() + rs := routes[service] + mu.RUnlock() + op, _ := httproute.Match(rs, method, uri) + return op +} + // HasRoute reports whether a service models a REST operation at this method and // URI. It answers "is this request one this service could serve?", which is how // the gateway tells apart services that share a SigV4 signing name and so cannot // be separated by the credential scope alone. func HasRoute(service, method, uri string) bool { - mu.RLock() - rs := routes[service] - mu.RUnlock() - op, _ := httproute.Match(rs, method, uri) - return op != "" + return Route(service, method, uri) != "" } // RegisteredOps returns the operation metadata registered for a service. From bd8e7f7518c6d669774f0286fba29b045be2304f Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sun, 6 Sep 2026 05:48:49 +0900 Subject: [PATCH 5/8] fix: the manifest reported an implemented operation as unimplemented MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RED (compile-time, the intended missing field): CGO_ENABLED=0 go test ./internal/codegen/ internal/codegen/gen_fidelity_test.go:83:4: unknown field ShortOperations in struct literal of type ProviderScan internal/codegen/scan_handverified_test.go:93:45: scans["resourcegroups"].ShortOperations undefined FAIL github.com/skyoo2003/devcloud/internal/codegen [build failed] The RED could not be committed on its own: .pre-commit-config.yaml runs go vet, which rejects a non-compiling tree, so RED and GREEN share this commit and the output above is the evidence. Same constraint as #144. GREEN: CGO_ENABLED=0 go test ./internal/codegen/ → ok CGO_ENABLED=0 go test ./... → all pass make codegen → exactly one tier change DEVCLOUD_BIN=dist/devcloud pytest tests/compatibility/ → 1126 passed, 17 skipped, 1 xfailed (was 2 xfailed) golangci-lint run → 0 issues opNamePattern requires four characters. resourcegroups implements Tag and Untag in the same switch; Untag was scanned, Tag was not, and the manifest called implemented code unimplemented. test_no_fabricated_success then asked for Tag, got the 200 the provider has always returned, and reported a fabricated success. It was never fabricated. The answer was real and the manifest was wrong, so the probe was right to complain and wrong about what it had found — which is why that entry recorded only an observation and refused to name a mechanism. Loosening the pattern is not the fix. At two characters it admits the HTTP verbs every path resolver switches on. The model separates them: Tag is an operation because resourcegroups.json says so, and no model AWS publishes declares GET. Short literals are collected into ProviderScan.ShortOperations and promoted by BuildFidelityData only where the service's model declares them. Fleet-wide, exactly one operation is short enough to be affected: rg -o '"[A-Z][A-Za-z0-9]{2}":\s+Tier[A-Za-z]+' \ internal/generated/fidelity/manifest_gen.go → 1 hit, "Tag" The gate caught the consequence, as designed: coverage_test.go:162: docs/coverage.md publishes 4496 hand-verified operations, the manifest holds 4497 coverage_test.go:162: docs/coverage.md publishes 2718 unimplemented operations, the manifest holds 2717 Plan: .claude/plans/aws-service-coverage-100-milestone-7.plan.md, Task 3 --- changes/unreleased/Fixed-20260906-170000.yaml | 5 ++ docs/coverage.md | 4 +- internal/codegen/gen_fidelity.go | 14 +++++ internal/codegen/gen_fidelity_test.go | 43 +++++++++++++ internal/codegen/scan_handverified.go | 60 +++++++++++++++---- internal/codegen/scan_handverified_test.go | 21 +++++++ internal/generated/compat/services.json | 1 + internal/generated/fidelity/manifest_gen.go | 2 +- .../test_no_fabricated_success.py | 27 ++++----- 9 files changed, 144 insertions(+), 33 deletions(-) create mode 100644 changes/unreleased/Fixed-20260906-170000.yaml diff --git a/changes/unreleased/Fixed-20260906-170000.yaml b/changes/unreleased/Fixed-20260906-170000.yaml new file mode 100644 index 00000000..4bf5c1c6 --- /dev/null +++ b/changes/unreleased/Fixed-20260906-170000.yaml @@ -0,0 +1,5 @@ +kind: Fixed +body: 'The fidelity manifest no longer drops operations with short names. The scan that decides which operations are hand-verified required a name of four characters or more, so `resourcegroups.Untag` was recorded and `Tag` — implemented beside it in the same dispatch switch — was reported as unimplemented. A probe then asked for `Tag`, got the answer the provider has always given, and reported a fabricated success that was never fabricated: the code was real and the manifest was wrong about it. Short dispatch literals are now collected separately and promoted only where the service''s own model declares them, so `Tag` counts and the `GET`/`PUT` literals every path resolver switches on still cannot become operations. Hand-verified operations: 4,496 to 4,497' +time: 2026-09-06T17:00:00.000000+09:00 +custom: + Issue: "148" diff --git a/docs/coverage.md b/docs/coverage.md index 17a0863f..79c2fe34 100644 --- a/docs/coverage.md +++ b/docs/coverage.md @@ -22,9 +22,9 @@ Per operation, from the [fidelity manifest](fidelity-manifest.md): | Tier | Operations | |---|---| -| `hand-verified` | 4,496 | +| `hand-verified` | 4,497 | | `auto-crud` | 5,193 | -| `unimplemented` | 2,718 | +| `unimplemented` | 2,717 | | **total known** | **12,407** | ## Why a registered service can serve nothing diff --git a/internal/codegen/gen_fidelity.go b/internal/codegen/gen_fidelity.go index 9326e610..b692ac05 100644 --- a/internal/codegen/gen_fidelity.go +++ b/internal/codegen/gen_fidelity.go @@ -91,9 +91,23 @@ func BuildFidelityData( hand[op] = true universe[op] = true } + declared := make(map[string]bool, len(modelOps[serviceID])) for _, op := range modelOps[serviceID] { + declared[op] = true universe[op] = true } + // A dispatch literal too short to be told from an HTTP verb by shape is + // an operation when, and only when, the model declares it. + // resourcegroups implements Tag and Untag in one switch and only Untag + // was long enough to be scanned, so the manifest called implemented code + // unimplemented. The model is asked here rather than in the scanner + // because this is where the model already is — the scan reads Go source, + // not Smithy. + for _, op := range provider.ShortOperations { + if declared[op] { + hand[op] = true + } + } for op := range crudOps[serviceID] { universe[op] = true } diff --git a/internal/codegen/gen_fidelity_test.go b/internal/codegen/gen_fidelity_test.go index b54e7864..d4888321 100644 --- a/internal/codegen/gen_fidelity_test.go +++ b/internal/codegen/gen_fidelity_test.go @@ -60,6 +60,49 @@ func TestBuildFidelityDataRequiresEngineWiring(t *testing.T) { "a provider that never reaches the engine serves nothing, whatever the CRUD registry says") } +// TestBuildFidelityDataPromotesShortDeclaredOperations covers the operation name +// too short for the scanner's shape test. +// +// opNamePattern requires four characters, so `Untag` is scanned and `Tag` is +// not, even though resourcegroups implements both in the same switch. The +// manifest called Tag unimplemented, a probe asked for it, got the 200 the +// provider has always returned, and reported a fabricated success that was +// never fabricated — the manifest was simply wrong about served code. +// +// Loosening the pattern alone is not the fix: at two characters it admits the +// HTTP-verb literals that path-resolver switches are full of. The model is what +// separates them. `Tag` is an operation because resourcegroups.json says so; +// `GET` is not an operation in any model AWS publishes. +func TestBuildFidelityDataPromotesShortDeclaredOperations(t *testing.T) { + modelOps := map[string][]string{"svc": {"Tag", "Untag", "ListGroups"}} + providers := map[string]ProviderScan{ + "svc": { + Operations: []string{"Untag", "ListGroups"}, + // Below the strict shape test. Only the model decides which of + // these is an operation. + ShortOperations: []string{"Tag", "GET", "PUT"}, + EngineWired: false, + }, + } + + data := BuildFidelityData(modelOps, map[string]string{"svc": "rest-json"}, providers, nil) + + assert.Equal(t, "HandVerified", tierOf(t, data, "svc", "Tag"), + "the model declares Tag and the provider dispatches it, so it is served") + assert.Equal(t, "HandVerified", tierOf(t, data, "svc", "Untag"), + "the long name was never in doubt and must not regress") + + for _, svc := range data.Services { + if svc.ServiceID != "svc" { + continue + } + for _, op := range svc.Ops { + assert.NotContains(t, []string{"GET", "PUT"}, op.Name, + "a short literal no model declares must not become an operation") + } + } +} + // TestBuildFidelityDataHandVerifiedBeatsWiring keeps the documented precedence: // a hand-written implementation always wins, and it does not depend on engine // wiring because such an operation never falls through to the engine at all. diff --git a/internal/codegen/scan_handverified.go b/internal/codegen/scan_handverified.go index 246782b8..2b0dab4d 100644 --- a/internal/codegen/scan_handverified.go +++ b/internal/codegen/scan_handverified.go @@ -21,6 +21,18 @@ import ( // switch with real operations (HTTP verbs, content types, protocol tokens). var opNamePattern = regexp.MustCompile(`^[A-Z][A-Za-z0-9]{3,}$`) +// shortOpNamePattern is the same shape at two or three characters — the length +// where a real operation name and an HTTP verb stop being distinguishable by +// shape. `Tag` is an operation of resourcegroups; `GET` and `PUT` are what a +// path resolver switches on, and every provider that routes by path has them. +// +// Literals matching this are collected into ProviderScan.ShortOperations and +// are not operations yet. BuildFidelityData promotes the ones the service's +// model declares, which is the only source that can tell the two apart. Without +// the split, resourcegroups.Tag was dropped while Untag beside it in the same +// switch was kept, and the manifest reported implemented code as unimplemented. +var shortOpNamePattern = regexp.MustCompile(`^[A-Z][A-Za-z0-9]{1,2}$`) + // pathRoutedOps lists the operations of providers that route on HTTP method and // path instead of an operation name, so no operation-name literal appears in // their HandleRequest for the scan to find. Keep in sync by hand when these @@ -70,6 +82,10 @@ var pathRoutedOps = map[string][]string{ type ProviderScan struct { // Operations are the operation names the provider dispatches by hand. Operations []string + // ShortOperations are dispatch literals too short to be told apart from an + // HTTP verb by shape. They become operations only where the service's model + // declares them; see shortOpNamePattern. + ShortOperations []string // EngineWired reports whether the provider hands the operations it does not // implement to the generic CRUD engine, by returning plugin.ErrUnhandledOp. // Only a wired provider's CRUD-shaped operations are actually served, so @@ -110,9 +126,11 @@ func ScanProviders(servicesDir string) (map[string]ProviderScan, error) { return nil, err } for id, recvType := range pkg.serviceTypes { + ops, short := pkg.dispatchedOps(recvType) result[id] = ProviderScan{ - Operations: opsFor(id, pkg.dispatchedOps(recvType)), - EngineWired: pkg.wiresCRUDEngine(recvType), + Operations: opsFor(id, ops), + ShortOperations: sortedSet(short), + EngineWired: pkg.wiresCRUDEngine(recvType), } } } @@ -125,12 +143,18 @@ func opsFor(serviceID string, scanned map[string]bool) []string { if declared, ok := pathRoutedOps[serviceID]; ok { return append([]string(nil), declared...) } - ops := make([]string, 0, len(scanned)) - for op := range scanned { - ops = append(ops, op) + return sortedSet(scanned) +} + +// sortedSet flattens a literal set into a stable slice, so a regeneration diff +// shows provider changes and nothing else. +func sortedSet(set map[string]bool) []string { + out := make([]string, 0, len(set)) + for k := range set { + out = append(out, k) } - sort.Strings(ops) - return ops + sort.Strings(out) + return out } // packageScan is one service package's providers, keyed by receiver type name. @@ -191,8 +215,11 @@ func scanServiceDir(dir string) (packageScan, error) { // provider switches in HandleRequest itself: sqs branches on protocol into // handleQueryRequest/handleJSONRequest, and kafka wraps handleRequest to // camelCase the response. -func (p packageScan) dispatchedOps(recvType string) map[string]bool { - ops := make(map[string]bool) +// It returns two sets: the literals that carry an operation's shape on their +// own, and the short ones only the model can confirm. +func (p packageScan) dispatchedOps(recvType string) (ops, short map[string]bool) { + ops = make(map[string]bool) + short = make(map[string]bool) visited := make(map[string]bool) var walk func(name string) @@ -203,13 +230,13 @@ func (p packageScan) dispatchedOps(recvType string) map[string]bool { } visited[name] = true - collectCaseLiterals(fn.Body, ops) + collectCaseLiterals(fn.Body, ops, short) for _, delegate := range delegateCalls(fn) { walk(delegate) } } walk("HandleRequest") - return ops + return ops, short } // wiresCRUDEngine reports whether a provider type opts into the generic CRUD @@ -252,7 +279,7 @@ func (p packageScan) wiresCRUDEngine(recvType string) bool { // Keyed is the point: `switch op { case "GetItem": }` dispatches, whereas a // tagless `switch { case remaining == 0: }` is control flow whose cases are // expressions, not operation names. -func collectCaseLiterals(body *ast.BlockStmt, ops map[string]bool) { +func collectCaseLiterals(body *ast.BlockStmt, ops, short map[string]bool) { ast.Inspect(body, func(n ast.Node) bool { sw, ok := n.(*ast.SwitchStmt) if !ok || sw.Tag == nil { @@ -264,8 +291,15 @@ func collectCaseLiterals(body *ast.BlockStmt, ops map[string]bool) { continue } for _, expr := range clause.List { - if lit, ok := stringLiteral(expr); ok && opNamePattern.MatchString(lit) { + lit, ok := stringLiteral(expr) + if !ok { + continue + } + switch { + case opNamePattern.MatchString(lit): ops[lit] = true + case shortOpNamePattern.MatchString(lit): + short[lit] = true } } } diff --git a/internal/codegen/scan_handverified_test.go b/internal/codegen/scan_handverified_test.go index bef430ba..45d50185 100644 --- a/internal/codegen/scan_handverified_test.go +++ b/internal/codegen/scan_handverified_test.go @@ -77,6 +77,27 @@ func TestScanProviders(t *testing.T) { } } +// TestScanProvidersCollectsShortLiteralsSeparately covers the names the shape +// test is too strict for. +// +// opNamePattern needs four characters. `Tag` has three, so resourcegroups — +// which implements Tag and Untag next to each other in one switch — had one of +// them scanned and the other silently dropped. They are collected here rather +// than admitted into Operations, because at this length the scan cannot tell an +// operation from the HTTP verbs a path resolver switches on. BuildFidelityData +// asks the model, which can. +func TestScanProvidersCollectsShortLiteralsSeparately(t *testing.T) { + scans, err := ScanProviders("../services") + require.NoError(t, err) + + assert.Contains(t, scans["resourcegroups"].ShortOperations, "Tag", + "Tag is dispatched by the provider and is three characters long") + assert.NotContains(t, scans["resourcegroups"].Operations, "Tag", + "the strict set keeps its shape test; promotion happens against the model") + assert.Contains(t, scans["resourcegroups"].Operations, "Untag", + "the four-character sibling was never in doubt") +} + // TestScanProvidersKeepsOperationsTheModelOmits is the regression that made the // scan drop real coverage: it used to intersect with the Smithy model, so every // operation a provider serves beyond its in-tree model vanished from the diff --git a/internal/generated/compat/services.json b/internal/generated/compat/services.json index 37427412..fb5831c9 100644 --- a/internal/generated/compat/services.json +++ b/internal/generated/compat/services.json @@ -8125,6 +8125,7 @@ "PutGroupConfiguration", "SearchResources", "StartTagSyncTask", + "Tag", "UngroupResources", "Untag", "UpdateAccountSettings", diff --git a/internal/generated/fidelity/manifest_gen.go b/internal/generated/fidelity/manifest_gen.go index 53a1edd1..86672349 100644 --- a/internal/generated/fidelity/manifest_gen.go +++ b/internal/generated/fidelity/manifest_gen.go @@ -10081,7 +10081,7 @@ var Services = map[string]Service{ "PutGroupConfiguration": TierHandVerified, "SearchResources": TierHandVerified, "StartTagSyncTask": TierHandVerified, - "Tag": TierUnimplemented, + "Tag": TierHandVerified, "UngroupResources": TierHandVerified, "Untag": TierHandVerified, "UpdateAccountSettings": TierHandVerified, diff --git a/tests/compatibility/test_no_fabricated_success.py b/tests/compatibility/test_no_fabricated_success.py index 9c659c90..a2c9db0b 100644 --- a/tests/compatibility/test_no_fabricated_success.py +++ b/tests/compatibility/test_no_fabricated_success.py @@ -54,33 +54,26 @@ def _unserved_probe(service_id, entry): return name, None -# Two fabricated successes that this file's fix does not reach, marked strict so -# they fail the moment they start passing rather than rotting into an accepted -# state. Only what was observed is recorded — the mechanism behind each is not -# the provider `default` branch that was fixed here, and has not been identified: +# Fabricated successes this file's own fix does not reach, marked strict so they +# fail the moment they start passing rather than rotting into an accepted state. # # s3.ListBucketAnalyticsConfigurations # GET /{Bucket}?analytics answers 200 with an empty body. S3's provider # default returns MethodNotAllowed, so this is not that branch. # -# resourcegroups.Tag -# PUT /resources/{Arn}/tags answers 200 echoing the request's Arn and -# Tags. The manifest lists the operation as unimplemented and the CRUD -# registry does not hold it; it holds GetTags at the same path under GET, -# and httproute.Match does compare methods (match.go:42), so the obvious -# explanation is ruled out rather than confirmed. -# -# Both are real violations of the guarantee and neither is fixed here: they are -# different defects that deserve their own diagnosis and their own change. +# resourcegroups.Tag was the second entry, and it was never a fabricated success. +# The scanner that decides which operations are hand-verified required a name of +# four characters or more, so `Untag` was scanned and `Tag` — implemented beside +# it in the same switch — was dropped. The 200 was a real answer from real code: +# the manifest was wrong about it, this probe believed the manifest, and the +# report inherited the error. The scanner now promotes a short literal the +# service's model declares, `Tag` is labelled hand-verified, and the operation is +# no longer offered here as unserved. KNOWN_UNFIXED = { ( "s3", "ListBucketAnalyticsConfigurations", ): "empty 200 from S3's sub-resource routing", - ( - "resourcegroups", - "Tag", - ): "engine-shaped echo for an operation the manifest calls unimplemented", } # Every service that is addressable, routable, and has an unserved operation to From e817778c135134a2029df8d1db99630f22e5a3ae Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sun, 6 Sep 2026 05:49:36 +0900 Subject: [PATCH 6/8] test: add reproducer for S3 serving unknown sub-resources as listings RED: CGO_ENABLED=0 go test ./internal/services/s3/ -run UnhandledBucketSubresource --- FAIL: .../analytics --- FAIL: .../lifecycle --- FAIL: .../inventory --- FAIL: .../encryption --- FAIL: .../metrics --- FAIL: .../versions --- FAIL: .../replication --- FAIL: .../object-lock "sub-bucket..." should not contain "ListBucketResult" "200" is not greater than or equal to "400" GET /{bucket}?analytics misses every bucket sub-resource check and falls through to listObjects, so botocore reads a 200 as a successful ListBucketAnalyticsConfigurations. Seven more sub-resources take the same path. The seven legitimate listing shapes in the same test already pass, and are there so the fix cannot be a guard that declines a real ListObjects. Recorded in .claude/tdd/query-providers-fabricate-success.tdd.md as one of the two remaining xfail(strict) violations. --- internal/services/s3/provider_test.go | 56 +++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/internal/services/s3/provider_test.go b/internal/services/s3/provider_test.go index d1662c4c..a065172d 100644 --- a/internal/services/s3/provider_test.go +++ b/internal/services/s3/provider_test.go @@ -484,3 +484,59 @@ func TestS3Provider_BucketNotification(t *testing.T) { assert.Equal(t, 200, resp.StatusCode) assert.Contains(t, string(resp.Body), "MyTopic") } + +// TestS3Provider_UnhandledBucketSubresourceDeclines covers the fall-through that +// answered a sub-resource nobody implemented with a bucket listing. +// +// GET /{bucket}?analytics misses every sub-resource check and lands on +// listObjects, which returns 200 and . botocore reads that as +// a successful ListBucketAnalyticsConfigurations — a fabricated success for an +// operation this provider does not serve, and the one thing docs/coverage.md +// calls absolute. The same fall-through served ?inventory, ?metrics, +// ?replication, ?lifecycle, ?encryption and ?versions. +// +// The real listings are asserted alongside, because the guard is only correct if +// it declines nothing a caller legitimately sends. +func TestS3Provider_UnhandledBucketSubresourceDeclines(t *testing.T) { + p := newTestProvider(t) + defer func() { _ = p.Shutdown(context.Background()) }() + + _, err := p.HandleRequest(context.Background(), "", httptest.NewRequest("PUT", "/sub-bucket", nil)) + require.NoError(t, err) + + unhandled := []string{ + "analytics", "inventory", "metrics", "replication", + "lifecycle", "encryption", "versions", "object-lock", + } + for _, sub := range unhandled { + t.Run(sub, func(t *testing.T) { + req := httptest.NewRequest("GET", "/sub-bucket?"+sub, nil) + resp, err := p.HandleRequest(context.Background(), "", req) + require.NoError(t, err) + assert.NotContains(t, string(resp.Body), "ListBucketResult", + "an unimplemented sub-resource was answered with a bucket listing") + assert.GreaterOrEqual(t, resp.StatusCode, 400, + "an unimplemented sub-resource must decline, not succeed") + }) + } + + // Every shape of a real listing still has to work. These are the query + // parameters ListObjects and ListObjectsV2 actually carry. + listings := []string{ + "/sub-bucket", + "/sub-bucket?prefix=a/", + "/sub-bucket?prefix=a/&delimiter=/", + "/sub-bucket?list-type=2", + "/sub-bucket?list-type=2&max-keys=10&fetch-owner=true", + "/sub-bucket?list-type=2&continuation-token=abc&start-after=k", + "/sub-bucket?marker=k&max-keys=5&encoding-type=url", + } + for _, uri := range listings { + t.Run("listing "+uri, func(t *testing.T) { + resp, err := p.HandleRequest(context.Background(), "", httptest.NewRequest("GET", uri, nil)) + require.NoError(t, err) + assert.Equal(t, 200, resp.StatusCode) + assert.Contains(t, string(resp.Body), "ListBucketResult") + }) + } +} From cde7db88762dd28bed411173d8298ceef6bf14e2 Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sun, 6 Sep 2026 05:52:21 +0900 Subject: [PATCH 7/8] fix: S3 served an unimplemented bucket sub-resource as a listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GREEN: CGO_ENABLED=0 go test ./internal/services/s3/ → ok (8 sub-resources decline, 7 listing shapes still serve) CGO_ENABLED=0 go test ./... → all pass DEVCLOUD_BIN=dist/devcloud pytest tests/compatibility/ → 1127 passed, 17 skipped, 0 xfailed make codegen; git status --porcelain internal/generated → clean bash scripts/generate-imports.sh; git diff --exit-code → clean golangci-lint run → 0 issues GET /{bucket}?analytics matched no sub-resource branch and fell through to listObjects, which answers 200 with . botocore reads that as a successful ListBucketAnalyticsConfigurations — a fabricated success, the one thing docs/coverage.md calls absolute. The recorded report named one sub-resource; the fall-through covered eight. The guard is an allow-list of the parameters a listing carries, not a deny-list of sub-resource names, because the two fail in opposite directions. A missing allow-list entry declines a real listing and a test says so at once; a missing deny-list entry serves a sub-resource as a listing, silently, which is the defect. A sub-resource AWS adds next year now declines on its own. The xfail(strict) marker did its job: it turned XPASS the moment the guard landed, so KNOWN_UNFIXED could not quietly keep an entry for something already fixed. Both entries are now gone and the dict is empty. Plan: .claude/plans/aws-service-coverage-100-milestone-7.plan.md, Task 4 --- changes/unreleased/Fixed-20260906-180000.yaml | 5 ++ internal/services/s3/provider.go | 60 +++++++++++++++++++ .../test_no_fabricated_success.py | 35 +++++------ 3 files changed, 83 insertions(+), 17 deletions(-) create mode 100644 changes/unreleased/Fixed-20260906-180000.yaml diff --git a/changes/unreleased/Fixed-20260906-180000.yaml b/changes/unreleased/Fixed-20260906-180000.yaml new file mode 100644 index 00000000..2a72d0f5 --- /dev/null +++ b/changes/unreleased/Fixed-20260906-180000.yaml @@ -0,0 +1,5 @@ +kind: Fixed +body: 'S3 no longer answers an unimplemented bucket sub-resource with a bucket listing. `GET /{Bucket}?analytics` matched no sub-resource branch and fell through to `ListObjects`, so botocore read the resulting 200 `` as a successful `ListBucketAnalyticsConfigurations`. `?inventory`, `?metrics`, `?replication`, `?lifecycle`, `?encryption`, `?versions` and `?object-lock` all took the same path. A bucket-level GET carrying a query parameter that is neither a sub-resource this provider serves nor a parameter a listing carries now returns a clean `NotImplemented`. The check is an allow-list of listing parameters rather than a deny-list of sub-resource names, so a sub-resource AWS adds later declines instead of being served as a listing' +time: 2026-09-06T18:00:00.000000+09:00 +custom: + Issue: "149" diff --git a/internal/services/s3/provider.go b/internal/services/s3/provider.go index f14818c6..50af336f 100644 --- a/internal/services/s3/provider.go +++ b/internal/services/s3/provider.go @@ -121,6 +121,23 @@ func (p *S3Provider) HandleRequest(ctx context.Context, _ string, req *http.Requ if _, ok := q["notification"]; ok { return p.getBucketNotification(ctx, bucket) } + // Everything above named a sub-resource this provider serves. What + // is left is either a bucket listing or a sub-resource nothing here + // implements, and falling through treated both as a listing — so + // GET /{bucket}?analytics answered 200 with and + // botocore read it as a successful + // ListBucketAnalyticsConfigurations. A fabricated success, and the + // same fall-through covered ?inventory, ?metrics, ?replication, + // ?lifecycle, ?encryption, ?versions and ?object-lock. + // + // The check is on the parameters a listing actually carries rather + // than on a list of sub-resource names, so a sub-resource AWS adds + // tomorrow declines on its own instead of being served as a listing + // until somebody notices. + if sub, unhandled := unhandledSubresource(q); unhandled { + return xmlError("NotImplemented", + "the "+sub+" sub-resource is not implemented", http.StatusNotImplemented), nil + } if q.Get("list-type") == "2" { return p.listObjectsV2(ctx, bucket, q) } @@ -413,6 +430,49 @@ type locationConstraintXML struct { // splitBucketKey splits a URL path like /bucket/key/parts into (bucket, key). // The leading slash is stripped before splitting. +// listObjectsParams are the query parameters a bucket listing carries. Two +// operations share the endpoint — ListObjects and ListObjectsV2 — and between +// them this is the whole set. +// +// An allow-list rather than a deny-list of sub-resource names, because the two +// fail in opposite directions. A missing entry here declines a legitimate +// listing parameter, which a test catches immediately; a missing entry in a +// deny-list serves a sub-resource as a listing, which is silent and is exactly +// the defect being fixed. +var listObjectsParams = map[string]bool{ + "prefix": true, "delimiter": true, "max-keys": true, "encoding-type": true, + "marker": true, // ListObjects + "list-type": true, // ListObjectsV2 + "start-after": true, "continuation-token": true, "fetch-owner": true, + "expected-bucket-owner": true, "optional-object-attributes": true, + // Not a parameter of either listing, but every SDK appends it to a + // presigned URL, and dropping the request on it would break every + // presigned listing. + "x-id": true, +} + +// unhandledSubresource reports the first query parameter that is neither a +// listing parameter nor a sub-resource this provider serves, which makes the +// request an operation S3 models and DevCloud does not implement. +// +// Sub-resources the provider does serve never reach here: their branches return +// before this is called. +func unhandledSubresource(q url.Values) (string, bool) { + names := make([]string, 0, len(q)) + for name := range q { + if !listObjectsParams[name] { + names = append(names, name) + } + } + if len(names) == 0 { + return "", false + } + // Sorted so the same request always names the same sub-resource, whatever + // order Go walked the map in. + sort.Strings(names) + return names[0], true +} + func splitBucketKey(path string) (bucket, key string) { path = strings.TrimPrefix(path, "/") if path == "" { diff --git a/tests/compatibility/test_no_fabricated_success.py b/tests/compatibility/test_no_fabricated_success.py index a2c9db0b..786a4e22 100644 --- a/tests/compatibility/test_no_fabricated_success.py +++ b/tests/compatibility/test_no_fabricated_success.py @@ -57,24 +57,25 @@ def _unserved_probe(service_id, entry): # Fabricated successes this file's own fix does not reach, marked strict so they # fail the moment they start passing rather than rotting into an accepted state. # -# s3.ListBucketAnalyticsConfigurations -# GET /{Bucket}?analytics answers 200 with an empty body. S3's provider -# default returns MethodNotAllowed, so this is not that branch. +# Empty. Both entries it carried are fixed, and each turned out to be a different +# defect from the one the entry guessed at: # -# resourcegroups.Tag was the second entry, and it was never a fabricated success. -# The scanner that decides which operations are hand-verified required a name of -# four characters or more, so `Untag` was scanned and `Tag` — implemented beside -# it in the same switch — was dropped. The 200 was a real answer from real code: -# the manifest was wrong about it, this probe believed the manifest, and the -# report inherited the error. The scanner now promotes a short literal the -# service's model declares, `Tag` is labelled hand-verified, and the operation is -# no longer offered here as unserved. -KNOWN_UNFIXED = { - ( - "s3", - "ListBucketAnalyticsConfigurations", - ): "empty 200 from S3's sub-resource routing", -} +# s3.ListBucketAnalyticsConfigurations was the bucket-listing fall-through. +# GET /{Bucket}?analytics matched no sub-resource branch and landed on +# listObjects, so an unimplemented sub-resource was answered with +# . Seven others took the same path. The provider now +# declines any bucket-level query parameter that is neither a sub-resource it +# serves nor a parameter a listing carries. +# +# resourcegroups.Tag was never a fabricated success. The scan that decides +# which operations are hand-verified required four characters, so `Untag` was +# recorded and `Tag` — implemented beside it in the same switch — was dropped. +# The 200 was real code answering; the manifest was wrong, this probe believed +# the manifest, and the report inherited the error. +# +# Both were recorded as observations with the mechanism explicitly unidentified, +# which is why neither had to be un-guessed before it could be fixed. +KNOWN_UNFIXED: dict[tuple[str, str], str] = {} # Every service that is addressable, routable, and has an unserved operation to # ask for. Built at collection time so the parametrisation names the operation From 60d2263842b15ead59ff91a62473a07f408fce24 Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sun, 6 Sep 2026 09:50:54 +0900 Subject: [PATCH 8/8] chore: point the changelog fragments at PR #146 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four Fixed entries were written before the PR existed and guessed consecutive numbers. custom.Issue carries the PR number in this repo — #142 through #145 all match their PR — and all four fragments ship in one PR. --- changes/unreleased/Fixed-20260906-160000.yaml | 2 +- changes/unreleased/Fixed-20260906-170000.yaml | 2 +- changes/unreleased/Fixed-20260906-180000.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/changes/unreleased/Fixed-20260906-160000.yaml b/changes/unreleased/Fixed-20260906-160000.yaml index 9dc73a9c..db35a0f9 100644 --- a/changes/unreleased/Fixed-20260906-160000.yaml +++ b/changes/unreleased/Fixed-20260906-160000.yaml @@ -2,4 +2,4 @@ kind: Fixed body: 'Three operations the fidelity manifest calls `hand-verified` are now reachable. `appsync.ListApis`, `eks.ListAccessPolicies` and `opensearch.ListApplications` were each implemented by a `case` clause in their provider, but the provider''s hand-written path resolver did not know the route boto3 sends — `GET /v2/apis`, `GET /access-policies` and `GET /2021-01-01/opensearch/list-applications` — so the operation arrived empty and the call answered `NotImplemented`. Each provider now falls back to its own model''s route table when its resolver recognises nothing, which recovers the operation instead of losing it. No published figure moves: these operations were already labelled served, and the label is now true' time: 2026-09-06T16:00:00.000000+09:00 custom: - Issue: "147" + Issue: "146" diff --git a/changes/unreleased/Fixed-20260906-170000.yaml b/changes/unreleased/Fixed-20260906-170000.yaml index 4bf5c1c6..b2144bd4 100644 --- a/changes/unreleased/Fixed-20260906-170000.yaml +++ b/changes/unreleased/Fixed-20260906-170000.yaml @@ -2,4 +2,4 @@ kind: Fixed body: 'The fidelity manifest no longer drops operations with short names. The scan that decides which operations are hand-verified required a name of four characters or more, so `resourcegroups.Untag` was recorded and `Tag` — implemented beside it in the same dispatch switch — was reported as unimplemented. A probe then asked for `Tag`, got the answer the provider has always given, and reported a fabricated success that was never fabricated: the code was real and the manifest was wrong about it. Short dispatch literals are now collected separately and promoted only where the service''s own model declares them, so `Tag` counts and the `GET`/`PUT` literals every path resolver switches on still cannot become operations. Hand-verified operations: 4,496 to 4,497' time: 2026-09-06T17:00:00.000000+09:00 custom: - Issue: "148" + Issue: "146" diff --git a/changes/unreleased/Fixed-20260906-180000.yaml b/changes/unreleased/Fixed-20260906-180000.yaml index 2a72d0f5..c6b2a918 100644 --- a/changes/unreleased/Fixed-20260906-180000.yaml +++ b/changes/unreleased/Fixed-20260906-180000.yaml @@ -2,4 +2,4 @@ kind: Fixed body: 'S3 no longer answers an unimplemented bucket sub-resource with a bucket listing. `GET /{Bucket}?analytics` matched no sub-resource branch and fell through to `ListObjects`, so botocore read the resulting 200 `` as a successful `ListBucketAnalyticsConfigurations`. `?inventory`, `?metrics`, `?replication`, `?lifecycle`, `?encryption`, `?versions` and `?object-lock` all took the same path. A bucket-level GET carrying a query parameter that is neither a sub-resource this provider serves nor a parameter a listing carries now returns a clean `NotImplemented`. The check is an allow-list of listing parameters rather than a deny-list of sub-resource names, so a sub-resource AWS adds later declines instead of being served as a listing' time: 2026-09-06T18:00:00.000000+09:00 custom: - Issue: "149" + Issue: "146"