Queue: snapshot data-loss revert, three accepted-then-ignored params, and a diff-scoped lint gate - #2417
Merged
Conversation
|
Important Review skippedToo many files! This PR contains 2321 files, which is 2221 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (2321)
You can disable this status message by setting the |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Collaborator
Author
📊 Code Coverage Report
Tip This project maintains a minimum coverage threshold of 85%. Maintain or improve coverage on new code to ensure long-term stability. Last updated: Sat, 15 Aug 2026 19:28:36 GMT |
added 16 commits
August 13, 2026 11:09
…dy could read These four services store and echo whole input maps, so a supplied value round-tripped fine and only presence went unchecked - the lowest tier of the required-member findings. Verifying that claim per field rather than trusting it turned up one place it was false. cloudwatchlogs ListAggregateLogGroupSummaries wrapped its response as logGroupSummaries, a key the real output shape does not have. Populated summaries never reached a real client, for every caller, whatever the groupBy. Found because the presence fix put someone in that handler. The op also declares ValidationException where most of the service uses InvalidParameterException, so it gets its own sentinel rather than the service's usual one. The audit undercounted three services. CreatePredictor requires three members, not the two named. comprehend CreateFlywheel also requires DataAccessRoleArn. quicksight CreateOAuthClientApplication also requires ClientId and ClientSecret - and those deliberately do not round-trip, since the real response shape has no such members, so their check is presence-only. forecast's table is keyed by action name rather than resource kind on purpose: CreatePredictor and CreateAutoPredictor share a kind, but the auto variant has no FeaturizationConfig field at all, so a kind-keyed table would have rejected valid calls. Fixing InputDataConfig also revived a nested foreign-key check that had been dead code, since no caller had ever supplied the field. Enum-valued members validate against the SDK's own Values() rather than a copied list. Thirty-four existing tests omitted these fields and asserted success. Closes gopherstack-wl0s
The prior checkpoint recorded what shipped. This one records what generalises, because the findings outlived the fixes. Chiefly: an A grade certifies op-level wire and routing, not field-level completeness - confirmed five times independently. Five manifests positively claimed verification that was false. Borrowed shapes and behaviour appeared at five distinct layers. Fifteen tests were wrong in the same direction as their bug, and a dedicated hunt for that pattern found zero while fixing bugs surfaced six, which says it is a checklist item rather than a backlog. Also records the scanner blind spots, since the scratchpad never survives and those tools were rebuilt from scratch four times.
…ions shape no real client can send CancelDomainConfigChange returned DescribeElasticsearchDomainConfig's DomainConfig envelope - a different operation's response entirely - instead of CancelledChangeIds, CancelledChangeProperties and DryRun. DryRun was never read. The existing unit test asserted the wrong shape and passed. The cancelled lists are empty here, since this backend applies changes synchronously and never holds a pending one. CreateVpcEndpoint and UpdateVpcEndpoint modeled VpcOptions as map[string]string where the real type carries SecurityGroupIds and SubnetIds arrays. A real client always sends arrays, so the unmarshal failed and CreateVpcEndpoint 400'd unconditionally for anything but a toy caller. Fixed by reusing the vpcOptions machinery handler_domains.go already had for the same SDK type at domain level. Both are the borrowed-shape class: one op wearing another's response, one field wearing another shape entirely. Noted for a later tightening pass, not a bug: three VPC endpoint ops return extra fields beyond the real summary shape, which restjson1 clients ignore. Refs gopherstack-p2mx
…hree levels off The issue said List returns the same item shape as Get. It does not - List is NARROWER. AnnotationImportJobItem and VariantImportJobItem have no items, formatOptions or statusMessage member at all, and this backend was marshaling the Get-shaped struct for both, leaking three fields into every List response. That is a new failure direction worth naming: an over-wide response cannot be caught by an SDK-driven test, because the deserializer silently discards keys it does not recognise. Those two tests inspect the raw body instead. Every other test here drives the real client, which remains the only proof for the opposite direction. Get genuinely does return ItemDetail, so the item types are now split: the Source-only shape stays for Start's input, and new detail types back Get. JobStatus is stamped from the job's own status at Start, honest here because this backend completes jobs synchronously in one step, so it is each item's true final state rather than a guess. ssm ListNodes turned out NOT to be the ListNodesSummary stub class - its real input has no required members, so an empty struct was defensible. It was still broken: all four optional members were discarded, so filtering and pagination never worked. Reading the whole operation found worse - the real Node element nests PlatformType and AgentVersion three levels down under NodeType.Instance, while this served them at the top level alongside a RegistrationDate that corresponds to no real field. Its existing test asserted the old top-level map, so it passed against the bug. Closes gopherstack-7s8r Closes gopherstack-6uag
…vice The five KeyValueStore ops had handlers in services/cloudfront but belong to a separate SDK module with its own path scheme, which cloudfront's /2020-05-31/ RouteMatcher can never match. They were unreachable by any real client. Registered rather than deleted, because the backing state was real: cloudfront's backend already had working per-store data and ETag maps and five correct methods. Only the front door was wrong. The new service borrows that backend and owns no state, mirroring how dynamodbstreams borrows dynamodb's. Verifying the wire shape against the real SDK found bugs the dead code had too: DescribeKeyValueStore must return the DATA-plane ETag, not the resource's control-plane one, or a real client's Describe-then-Put workflow breaks on the required IfMatch. And an ETag mismatch is ConflictException, not the 412 the old handlers used - that status does not exist in this SDK's error model. The KVS maps were also absent from cloudfront's snapshot, so they were dropped across restart. Added. Graded B honestly: real SDK round-trip tests exist, including the EndpointResolverV2 override this SDK needs because its ruleset derives a virtual host from the ARN, but there is no Docker integration suite yet. Closes gopherstack-4ara
The guard in pkgs/persistence has been failing on this branch since the rds retype landed, and three additive changes after it compounded the drift - cloudfront's KVS maps, iam's CurrentPassword and GlobalEndpointTokenVersion, kinesis's MinimumThroughputBillingCommitment. All four are legitimate and were verified individually before refreshing: rds is a genuine incompatible retype that the guard correctly classified as such, and the other three are additive with no bump, which is exactly right. I missed this by gating each change against only the services it touched. This test lives in pkgs and fires on changes to any of them, so a scoped gate cannot see it. Cross-cutting tests need a repo-wide run before pushing. Refs gopherstack-5i6p
…nt could reach The over-wide leaks were the reported bug; reading the operations found worse sitting underneath two of them. iot ListPackages and ListPackageVersions wrapped their results under packageList and packageVersionList - keys the real outputs do not have. A real client's list was ALWAYS EMPTY whatever the backend held, so the leaked fields never reached anyone anyway. ListCommands also tagged its timestamp creationDate where the wire key is createdAt. iot ListCommandExecutions was worse still: its real route is POST /command-executions with filters in the body, and the RouteMatcher never matched the bare path, so a real client 404'd and never reached the wrong field name that was reported. Fixed both, keeping the old fictional nested route for compatibility. CompletedAt and StartedAt stay absent - this backend has no device execution flow to source them from. The five over-wide responses are now scoped to the real Summary types, following patterns each service already had right elsewhere: iot's ListCertificates and ListThingTypes, and quicksight's ListIAMPolicyAssignmentsForUser and ListAssetBundleImportJobs. backup's shared restoreJobToJSON emitted ResourceArn where both real types use SourceResourceArn, so Describe and both List variants were wrong together. The two bug classes needed opposite test techniques: raw-body assertions for the over-wide leaks, since an SDK client silently discards unrecognised keys, and real-client round trips for the wrong names, since a raw body cannot show what a client actually loses. Closes gopherstack-g3jk Closes gopherstack-k26u
… route
The RouteMatcher matched /command-executions/{executionId}, but
resolveFinalOpsGroupB had only a DELETE case for that prefix. The op resolved
to unknownOperation and a real client got a 400 before reaching the handler.
The failure sits between matching and resolving, which is why the service's
own route test could not catch it: TestRouteMatcher_ExhaustiveCoverage calls
matchIoTPath for every real SDK path and asserts only that the path matches.
It never varies the method, never calls resolveOperation, and never dispatches
a request - so it passed against this bug with no warning.
Reading the operation found a second bug: both this route and the legacy
nested one serialised the raw struct, whose tag is thingArn - a member
GetCommandExecutionOutput does not declare, the real key being targetArn. Both
now go through the scoped map ListCommandExecutions already used.
Real GetCommandExecution addresses an execution by executionId and targetArn
with no commandId, which the existing backend lookup could not serve, so it
gains a by-id variant mirroring how DeleteCommandExecution already works.
Seven output members stay absent and documented - no control-plane op exists
to source any of them.
The rest of the family checked out: six other command ops are correctly wired,
and this was the last unreachable one.
Closes gopherstack-8ez0
Both asserted the old wrong behaviour, so they failed once production was right. Verified against the pinned SDKs that production is correct and the tests were stale, rather than assuming it from the commit messages. TestLambda_CapacityProvider posted Name and TargetOnDemandConcurrency. The real input requires CapacityProviderName, PermissionsConfig and VpcConfig, and TargetOnDemandConcurrency appears nowhere in the operation. The response assertions were wrong too - State and LastModified, with a title-cased Active. TestIntegration_OpenSearch_DomainLifecycle used one basePath for four calls. Create, Describe and Delete do live under /2021-01-01/opensearch/domain, but ListDomainNames is the un-prefixed /2021-01-01/domain - which is why a single variable cannot serve them and why the routing fix broke the test. Split into two paths rather than reintroducing one. These are the sixteenth and seventeenth tests found today encoding the same assumption as a bug, and the first two outside a service package. Service-scoped work never runs test/integration, which is how they survived - the same blind spot that left the pkgs/persistence guard red. A sweep for other stale integration tests covering today's fixes found none: most of those operations have no integration coverage at all.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| func route(r *http.Request) (string, string, string) { | ||
| segs := rawPathSegments(r) | ||
| if len(segs) < segCountStore || segs[0] != "key-value-stores" { |
|
|
||
| func route(r *http.Request) (string, string, string) { | ||
| segs := rawPathSegments(r) | ||
| if len(segs) < segCountStore || segs[0] != "key-value-stores" { |
added 10 commits
August 13, 2026 13:38
These tests asserted that ExtractOperation returns the right operation name for
every SDK-derived method and path. That is one stage past where the iot bug in
gopherstack-8ez0 failed - but ExtractOperation is an observability hook for
metrics labels, not the dispatch contract, so an op whose name resolved
correctly while its dispatch had no matching case would still pass.
Each now also drives h.Handler()(c) on the same request and asserts the
response is not that service's unmatched-route sentinel. The mirror-tree
services went first - lambda, opensearch and route53 keep a hand-duplicated
extraction tree that only discipline holds in sync with real dispatch.
Every sentinel was established by reading the actual dispatch fallback and
confirming it is never emitted for a legitimate business-logic 404. They differ
more than expected: literal route-not-found strings, NoSuchOperation,
UnknownOperationException, and for the two RESTRouter services a bare {} body
at 404 or a 400 wrapping ResourceNotFoundException.
apigatewayv2 is deliberately left alone. Its route-miss fallback is
byte-identical, status and body, to dozens of legitimate resource-not-found
responses, so any assertion would either be unsound or need backend fixtures
for every path family. An unsound check here would be worse than none - this
session has already found seventeen tests that passed while their bug survived.
No strengthened test failed, matching the earlier audit that drove 590 ops
through the real handler and found zero drift.
Closes gopherstack-ey26
sesv2 SendBulkEmail called SendEmail with empty subject and body, ignoring the required DefaultContent - every bulk email was stored blank. It now resolves the template, inline or by name, and applies per-recipient replacement data over the defaults, reusing the substitution logic that already existed. appstream CreateThemeForStack read only StackName, dropping all four other required fields. Favicon and logo URLs are derived the way amplify and serverlessrepo already derive theirs, since the real Theme type carries URLs rather than raw S3 locations. rds RestoreDBInstanceFromS3 and RestoreDBClusterFromS3 each dropped three required members - and reading the whole operation showed three more were read but never validated. All seven per op are now checked. The three S3-source members have no field on the real response shapes, so they validate without persisting. redshift CreateHsmConfiguration never passed either HSM secret to the backend. Both are credentials, so this follows the service's own precedent: CreateCluster validates MasterUserPassword in the handler and never stores it. Neither secret is logged, stored or echoed, and every test now asserts they do not appear in a response. accessanalyzer CreateAccessPreview ignored Configurations, the thing being previewed. Stored opaquely, since nothing here interprets the union's content. One near-miss caught by reading the SDK types rather than by lint: Configurations was first echoed on both Get and List, but ListAccessPreviewsOutput returns a summary type that has no such member. Fixed, with a regression test asserting List never emits it. Seventeen test cases encoded these bugs - most asserted only a status code, and one sent an empty configurations map on every case. Closes gopherstack-afi1
…ary types Every List op reused the Get op's converter unscoped, so each emitted members the real Summary type never declares. ListSolutions and ListSolutionVersions leaked nine apiece. ListSolutionVersions is the telling one: a correctly-scoped converter already existed, built for a nested field elsewhere, and the List handler simply was not calling it. Each Summary type was read from the SDK separately rather than deriving one shape and applying it sixteen times - which caught RecommenderSummary, the one type in the set that legitimately retains a nested config object where every sibling drops it. Applying the pattern by analogy would have stripped a real field. Checking the inverse found FailureReason declared on eight Summary types with no backend field to source it, so those stay absent and documented. One had an honest source - DatasetGroup already carries the field - and now emits it conditionally. TWO FALSE CLAIMS REMOVED. The ListSolutions converter's comment asserted correctness while addressing only one of nine leaked members. Worse, PARITY.md carried a standing note titled 'Extra fields on List summaries are harmless', arguing the premise correctly - SDK clients do ignore unknown keys - and drawing the wrong conclusion from it. That note is why this passed several prior audits. Replaced, with the reason spelled out: raw-body and non-SDK callers see the leak. The tests read the raw recorder body rather than going through an SDK client, because a client silently discards unrecognised keys and would pass against all sixteen bugs. Four were hand-reverted to confirm the subtests fail. Closes gopherstack-sm02
awsconfig GetAggregateResourceConfig decoded into an empty input and returned whichever resource came first, so every distinct request got the same arbitrary item. Now resolves both required members. directoryservice is the clearest case of a dropped member hiding a wrong shape. DescribeCAEnrollmentPolicy returned a fabricated nested CAEnrollmentPolicy envelope that does not exist on the real API; the real shape is flat, with a six-value status enum, a status reason, directory id, timestamp and the PcaConnectorArn that was reported missing. Rebuilt. The persisted value changes from bool to a struct pointer, which genuinely cannot decode, so the snapshot version bump is the legitimate kind - verified the guard rejects it unbumped. cognitoidp decoded a SecretHash the real API does not have, and both sibling ops returned wrong shapes - a flat string and a string list where the real type is ClientSecretDescriptorType. Secrets are never logged and the list never returns values. kafka UpdateRebalancing dropped CurrentVersion and Rebalancing.Status behind a comment asserting AWS exposes no such setting. It does. Comment corrected, and two clone functions that would have silently dropped the new field were caught. eventbridge ListPartnerEventSourceAccounts was filed as possibly manifest-only. It is not: CreatePartnerEventSource already stores the offered account and mirrors its state, so the 'not simulable' premise was false and the op is now implemented for real. codeartifact PublishPackageVersion reads AssetSHA256 from the header, not the body - and the issue's cited MismatchedSha256Exception does not exist on this op, so mismatch returns ValidationException from its own declared set. apigatewayv2 ExportApi honours OutputType; lakeformation and guardduty validate their required members. Seven tests encoded these bugs. Closes gopherstack-h910
…s, and correct two bad findings Seven of the nine reported leaks were real: glue's three schema-registry ops, opensearch's two VPC-endpoint ops sharing one root cause, medialive ListSignalMaps, bedrock ListModelImportJobs and eks ListInsights. TWO OF MY FINDINGS WERE WRONG, both caught by reading each Summary type separately rather than trusting the issue. I listed tags among ListSignalMaps' leaks. SignalMapSummary genuinely declares Tags - confirmed in the deserializer. Stripping it would have introduced a bug while fixing one. ListChannelPlacementGroups is not over-wide at all. Its real backing type is DescribeChannelPlacementGroupSummary, which carries the same seven members as the Describe, Create, Update and Delete outputs - all four verified independently. The existing converter was already correct, and PARITY.md had recorded that from a prior pass. No change made. Reading the SDK also turned up a leak nobody had listed: eks emitted clusterName on both List and Describe, and neither the summary nor the full Insight type carries it on the wire - the cluster is already identified by the path. Fixed on List; Describe shares the older converter and is recorded as a separate pre-existing gap. Inverse direction: InsightSummary declares KubernetesVersion and Name with no honest source in the domain model, so both stay absent and documented. opensearch's StatusUntil is worth noting - an internal clock field driving time-based state transitions, never meant to reach the wire at all. Tests assert raw bodies, since an SDK client discards unrecognised keys and would pass against every one of these. eks needed a white-box companion because its backend never populates the leaked field on synthesised data. Closes gopherstack-uult
Zero bugs, and every check run rather than skipped. The service already held PARITY grade A from gopherstack-b9mg and a dedicated router test from gopherstack-jqh2; both were re-derived independently rather than trusted, which is the standard after twelve in-repo claims failed this week. What was actually verified: all 23 List and Get output shapes plus every nested type diffed field by field; both shared converters checked against the real API's own type-sharing, no sibling trap; required members diffed in BOTH directions across all 12 request bodies; all 20 filters across 8 List ops confirmed reaching the query; all 7 void ops confirmed genuinely empty on the real API rather than the appconfig StopDeployment trap; no discarded inputs, with ValidateOnly and DryRun both honored; router confirmed a real path-segment router, not immune, and all 43 ops reachable; 43 of 43 ops real. Protocol confirmed by reading all 235 EqualFold sites - the 57 without errorCode are NaN and Infinity float-literal matches, not body keys. Twenty-seventh service, twenty-seventh distribution. Credential sweep done deliberately and clean: ServerPublicKey is synthetic and non-cryptographic. ONE ITEM FLAGGED RATHER THAN RESOLVED, correctly. PARITY claims ListBlockingInstancesForCapacityTask is always empty because capacity is additive-only. The code matches that claim, but whether real AWS treats InstancePools as a delta or an absolute target CANNOT be determined from the pinned Go SDK alone. Recorded in the remainder file rather than trusted or quietly changed - the right answer when the evidence is genuinely unavailable. 87 of 162 swept, 75 remain. Refs gopherstack-6flj
… real client DeletePackageVersions, CopyPackageVersions, DisposePackageVersions and UpdatePackageVersionsStatus all built failedVersions and successfulVersions as JSON ARRAYS. The real shapes are MAPS keyed by version string. A real SDK client's call to any of the four died on deserialization - reproduced verbatim against unfixed code. One shared converter, four total outages. Two riders inside that same fix. An invented enum value RESOURCE_NOT_FOUND where the real one is NOT_FOUND - and DisposePackageVersions, immediately adjacent, already had it right. A sibling trap in reverse: the correct code was the neighbour, not the copy. Plus two fabricated status literals that are not enum members at all. DeletePackage reused the PackageDescription converter where the real output is a PackageSummary, dropping the identifier and leaking three Describe-only fields. Second shared-converter bug in one service. Also: four backend-tracked repository members never emitted across 8 ops; two ignored filters, including repository-prefix on two List ops; and two ops that never rejected a missing required policy document. THE TWO RAW-BODY TESTS HERE ARE DELIBERATE, not laziness. A real client structurally cannot send a request omitting a required field, so the only way to exercise those two validation gaps is below the SDK. Stated rather than glossed. All 9 distinct fixes hand-reverted individually and confirmed to fail with the predicted symptom. Seven ratifying tests rewritten - forty-three found. 88 of 162 swept, 74 remain. Refs gopherstack-6flj
…bers LastUpdateDateTime and FailureException had zero grep hits anywhere in the service - not mis-keyed, never modeled at all. LastUpdateDateTime is fixed and emitted only when non-zero, so a table whose insights were never toggled reports it ABSENT rather than a fabricated epoch zero. FailureException is disclosed rather than fixed: this backend's insights toggle cannot fail, so always-nil is accurate, not a gap. Before deciding not to propagate the field to the List item shape, the agent confirmed ContributorInsightsSummary genuinely lacks that member - checking rather than assuming symmetry between Describe and List. 21 of 22 wrapper keys were already correct, each diffed against its own Output struct. The shared exportTableToPointInTimeOutput converter was checked and is LEGITIMATELY shared - both real Outputs are identical. GlobalTableDescription's three call sites likewise confirmed genuinely separate wire types rather than a mismatched converter. That check has found two real bugs in the last two services, so a clean result from it is worth recording. Persistence handled correctly: Table doubles as the snapshot DTO, and the new field has a fresh tag rather than a retag, so old snapshots restore without a version bump. The prior PARITY grade A is not contradicted here - its deep-audit notes simply never covered this admin, List and Describe family. A coverage gap, not a note arguing a bug away. Closed with a new family entry. Router is a flat X-Amz-Target switch and structurally immune to the desync that 404'd two elasticsearch ops - stated rather than checked op by op, which is the correct shortcut for that dispatch style. The one lint finding was fixed with gofmt rather than fieldalignment -fix, deliberately avoiding the nolint-stripping hazard. 89 of 162 swept, 73 remain. Refs gopherstack-6flj
… client It reused Get's response shape - scanningConfiguration plus registryId - where the real Put output wraps under registryScanningConfiguration and has no registryId at all. A real client always got nil back. IT HID BEHIND A SYMMETRIC-LOOKING GET/PUT PAIR FOR THREE PRIOR AUDIT ROUNDS. That is the shared-converter class again, but disguised: Get and Put reading as mirror images is exactly why nobody diffed them separately. An existing raw-body test asserted the wrong key and is rewritten. registryId was declared and never populated on four more ops, while sibling ops already got it right - and PutSigningConfiguration correctly has none, verified per op rather than assumed uniform. DescribeRepositoryCreationTemplates discarded maxResults and nextToken entirely, always returning everything in one page. BatchGetRepositoryScanningConfiguration omitted appliedScanFilters. DescribeImageScanFindings leaked five top-level-only fields by reusing the internal domain struct wholesale. THE BEST DECISION HERE WAS NOT SHIPPING A FIX. ListImageReferrers is missing three real members, so the agent built the fix, wrote a test, hand-reverted - AND THE TEST STILL PASSED. PutImage never builds referrer relationships, so the op is structurally always empty and nothing could observe the change. Both were reverted and the real gap recorded in PARITY instead. That is the seventh worthless test the revert step has caught, and the first caught before it ever entered a diff. Tie broken on sibling-trap surface as briefed: neptune has 10 resource-family handler files, ecr has 14. Credential sweep clean - the auth token is synthetic. 58 of 58 ops real, all 274 EqualFold sites inspected rather than counted. 90 of 162 swept, 72 remain. Refs gopherstack-6flj
All 21 List, Describe and Get ops diffed individually against their own real deserializer. Every wrapper key already correct, including the non-obvious DescribeEventSubscriptions returning EventSubscriptionsList rather than EventSubscriptions. All three shared converters - covering 4, 6 and 7 call sites - confirmed LEGITIMATELY shared, each wrapping one identical real type everywhere. That check found a four-op outage and a nil-returning Get/Put pair in the two preceding services, so a clean result from it is evidence rather than absence. Two members never modelled at all. EventSubscription.CustomerAwsId was a pure threading gap - the backend already had accountID. GlobalCluster.DatabaseName is a real optional Create input that was never read or echoed. Two things disclosed rather than faked. GlobalCluster.FailoverState is genuinely transient and this backend's failover applies synchronously, so there is no window to report honestly. And CreateGlobalCluster discards three real inputs whose validation surface deserves its own pass rather than a hurried fix. THE TIE-BREAK IS WORTH RECORDING BECAUSE IT CONTRADICTS THE BRIEF. I told the agent to break ties on sibling-trap surface; ecr has 14 handler families to neptune's 10, so surface pointed at ecr. A sibling was already mid-edit there, so OCCUPANCY decided it - and the agent said so plainly rather than retrofitting the surface rationale it had been handed. The prior grade-A audit is accurate everywhere it looked and simply never looked at these three members - a coverage gap, not an argued-away bug. It also caught that PARITY's last_audit_commit points at an unrelated sesv2 commit, likely stale. Persistence checked: both structs are their own snapshot DTOs, both fields added with fresh tags, snapshot version left at 1. One fieldalignment finding fixed BY HAND rather than with -fix, per the nolint-stripping hazard. 91 of 162 swept, 71 remain. Refs gopherstack-6flj
… types Zero wrapper-key or nesting bugs. All 20 List, Describe and Get keys plus all 23 shared nested types diffed key-for-key against the real deserializer switches - and the key-set extraction was SCRIPTED rather than hand-transcribed, which matters at that width. Two members never modelled and both DISCLOSED rather than fixed. AwsDevice across three types and VirtualGatewayRegion are real in the deserializer with zero grep hits here, but both are marked Deprecated in the SDK's own doc comments and nothing available confirms whether real AWS still populates them. Recorded in PARITY gaps rather than guessed - the evidence to decide simply is not there. THE TIE-BREAK INVERTED THE PRECEDENT AND THE AGENT SAID SO. Surface pointed at xray, 14 handler families to directconnect's 6, so it started on xray. Partway through a read-only investigation a sibling's uncommitted xray changes appeared, so occupancy overrode surface mid-stream and it switched cleanly, having only ever read. Last pass had surface pointing at an occupied service from the start; this time surface picked correctly first and occupancy corrected it later. The credential sweep is the useful negative here: AuthKey and Ckn both match real AWS's own wire shape, so their presence is parity rather than over-exposure. Worth distinguishing after three genuine leaks earlier in this sweep. The prior grade-A audit is a coverage gap, not a false claim - it worked from Go struct definitions and never read the deserializer's key switch case by case. Its last_audit_commit is also stale, resolving to an unrelated cross-service commit. Second stale audit pointer found in two passes. Router confirmed structurally immune rather than assumed. 64 of 64 ops real, all 157 EqualFold hits scoped to error codes. Required-field diffing scoped honestly to the 20 touched ops, and noted that this SDK ships zero client-side validators for the whole service. 92 of 162 swept, 70 remain. Refs gopherstack-6flj
…otation
Annotations was emitted as a flat map of scalars. The real shape is a map of
ARRAYS of tagged-union objects, and the real deserializer type-asserts on
[]interface{} and hard-errors 'unexpected JSON type' on anything else. So every
real client call against a trace carrying at least one annotation died - not
silent-empty.
IT SURVIVED A THOROUGH DEDICATED AUDIT that had already fixed several wrapper-key
bugs in this same service by the same method. That pass diffed member names and
nesting but never checked the Go KIND of a map-of-collections value. A coverage
gap on a different axis rather than a wrong claim - which is why the kind check
is now its own checklist item and not a footnote under naming.
GetInsightSummaries parsed GroupARN, GroupName, StartTime and EndTime and passed
none of them to the backend, so every group and every time window returned the
same unfiltered set. The tractable layer is fixed - validation plus group and
time filtering - and the rest is DISCLOSED rather than papered over: the
detector labels every insight 'default' with no per-group filter-expression
evaluation, so a request for 'default' still gets everything. Op downgraded from
ok to partial.
Sampling and SamplingStrategy on the same op are a no-op safe superset, recorded
rather than faked.
Shared converters all checked and clean, including a Get/Put EncryptionConfig
pair confirmed a GENUINE symmetric pair rather than the trap that made ecr's
PutRegistryScanningConfiguration return nil. Symmetry is a suspect, not a
verdict.
Both agents converged on the same tie-break independently - 14 handler families
against 6 - and the other switched away when it saw these edits mid-flight.
Confirmed from both sides, no collision.
93 of 162 swept, 69 remain.
Refs gopherstack-6flj
…silently dropped It was wire-tagged at the top level of StartMedicalScribeJob's input and of the job shape, but the real SDK only carries it nested under Settings. The real deserializer's default case discards unknown top-level keys without error, so the setting was accepted, ignored, and never surfaced - no failure anywhere for a client to notice. An existing test asserted the wrong placement as correct. All 19 wrapper keys were already right. Every bug this pass was one level deeper: two shared VocabularyInfo ops missing LastModifiedTime, a CallAnalyticsSettings member with zero grep hits and distinct from the same-named field already fixed at job level, and all four Call Analytics rule filter types missing both of their time-range members. Key sets were SCRIPT-EXTRACTED from the deserializer for all 19 ops and every reachable nested type - the width is where hand-transcription fails. THE TIE-BREAK INVERTED AGAIN AND THE AGENT SAID SO. mediatailor is wider by handler families, 12 to 9, so surface-first would have picked it. A sibling's files were already modified at pickup and a new test file appeared there BETWEEN two git status calls mid-session, so occupancy overrode surface. Fourth pass running to report the real cause rather than the briefed one. Two shared converters confirmed genuinely symmetric, and a VocabularyFilterInfo-versus-GetVocabularyFilterOutput pair confirmed an INTENTIONAL asymmetry already modelled correctly - checked rather than flattened to match. One over-modelled field left in place and disclosed: NonTalkTimeFilter carries a ParticipantRole the real type does not have, unreachable by any real client. Hand-reverts were done by editing the pre-fix shape back by hand, since git checkout is banned for agents this session, then verified byte-identical by index hash. Worth noting because awsjson1.1 tolerates unknown fields, so none of these produced a decode error - every confirmation was a nil or missing value. 94 of 162 swept, 68 remain. Refs gopherstack-6flj
GetFunction and PutFunction never emitted CustomOutputConfiguration,
HttpRequestConfiguration or SequentialExecutorConfiguration - the whole config
surface of that feature was unreachable to any client. ListFunctions dropped
those plus Description, because its Items is the same full type GetFunction
returns.
Four more List ops were dropping real per-item fields, ListChannels shedding 6
of 12 while the summary type already tracked every one. And ListLiveSources'
backend never populated its timestamps at all, where ListVodSources did -
verified per op rather than assumed uniform.
TWO FABRICATED FIELDS DETECTABLE ONLY IN THE RAW BODY: CreateChannel and
UpdateChannel emitted a LogConfiguration that neither real Output type has, and
the prefetch-schedule ops emitted a top-level CreationTime with no real member
at all. A typed client cannot see either, so both tests are deliberately
raw-body. One existing test had asserted the fabricated CreationTime as correct.
The symmetric pair here was a REAL asymmetry in both directions and both
directions were wrong: the List item type has LogConfiguration and no
TimeShiftConfiguration, the Create and Update outputs have the opposite.
Diffing them separately is the only way that surfaces.
Two stale PARITY notes corrected, both the argued-away kind - each ASSERTED a
field had been added and grep says otherwise.
THE BEST RESTRAINT WAS A FIX NOT ATTEMPTED. The agent nearly derived
ScheduleAdBreaks from Program.AdBreaks, then re-read PARITY's own note
explaining why that derivation is exactly the fabrication this issue warns
against, and left it. A second candidate, Audiences, has a plausible derivation
that is unconfirmed - disclosed rather than guessed.
Key extraction was scripted with a paren-balance walker, because a naive
function-body search breaks on this SDK - interface{} appears in the signature
before the real body opens.
95 of 162 swept, 67 remain.
Refs gopherstack-6flj
…eters was unusable SCRIPTING THE REQUEST SIDE IS THE HEADLINE. Both directions were script-walked - deserializers for responses, serializers for requests - and the request script caught two of the seven bugs. The prior audit here scoped itself to deserializers only, by its own stated method, so a response-only sweep could not have found them. Both multi-region parameter ops read their name filter under ParameterGroupName where the real key is MultiRegionParameterGroupName. A different key, not a casing variant, so this service's case-insensitive decode did not save it: on the required-field op EVERY real client's request failed with a 400, and on the optional one the filter was silently ignored. The response side of the same op tagged its list Parameters where the real key is MultiRegionParameters - a sibling trap, since the plain DescribeParameters genuinely does use Parameters. Cluster.IpDiscovery was tagged IPDiscovery through a shared object, so six ops were wrong. The request-side tags with the same spelling were checked and DELIBERATELY LEFT - encoding/json's case-insensitive fallback still binds them, so changing them would be churn. Same string, opposite verdicts, decided by which direction the decoder runs. MultiRegionCluster was missing NumberOfShards on the response while its source NumShards was a discarded input not even present in the request struct - the same bug from both sides at once. Pagination was discarded on 7 of 15 Describe ops. Six fixed. DescribeEvents DISCLOSED RATHER THAN FIXED because its result order is not deterministic across calls, so a cursor on top of it would be unsound - and the underlying region-scoping defect is flagged separately rather than buried under a pagination fix. Three more gaps disclosed, including DescribeUsers filters where AWS's own doc comment enumerates no valid Name values to implement against honestly. Eight of nine hand-reverts produced only a missing-value signal rather than an error, since awsjson1.1 tolerates unknown fields. Only the required-field request-key revert produced a hard 400. 96 of 162 swept, 66 remain. Refs gopherstack-6flj
…l client The output was tagged lowercase 'tags' where the real deserializer switches on case-sensitive PascalCase Tags and NextToken. This is the one op family in an otherwise camelCase service that uses AWS's shared generic tagging shape - the service convention was the trap. THE TWO EXISTING TESTS WERE STRUCTURALLY BLIND, not merely passing against unfixed code. Both decoded the response into a matching lowercase local struct using plain encoding/json, so the test and the handler agreed with each other in a way no amount of running them could expose. That is a stronger failure than the ratifying tests found so far. The request-side half of the same fix turned out NON-OBSERVABLE - the router's encoding/json already bound it via case-insensitive fallback - and is reported as such rather than counted. Same distinction memorydb drew last pass. Three more fixes, all DERIVED rather than invented: two deployment-group timestamps and a target revision built from real per-group history, an instance ARN following an existing format precedent in the same file, and a status message taken verbatim from the SDK's own doc comment. SIX GAPS DISCLOSED AND DELIBERATELY NOT ADDED, with a good argument: omitempty makes a present-but-always-empty field byte-identical on the wire to an absent one, so modelling them would be zero-effect churn rather than a fix. Recorded separately from the four real ones. The fieldalignment ordering was derived by running -fix against an ISOLATED SCRATCH COPY and applying it by hand, keeping the nolint-stripping hazard away from the real file. Neatest handling of that trap yet. Tie-break reported honestly again: memorydb had the widest surface at 12 handler families but was occupied, and between the two free services surface decided cleanly at 10 against 8. 97 of 162 swept, 65 remain. Refs gopherstack-6flj
…s archived everything
Three List ops - ListFindings, ListFindingsV2 and ListAccessPreviewFindings -
silently dropped the real filter wire key, so every finding came back regardless
of the criteria asked for. Found by grepping for discarded typed parameters,
which is now the twenty-ninth instance of that class.
BUILDING THE FILTER HELPER EXPOSED A WORSE BUG BESIDE IT. CreateArchiveRule and
ApplyArchiveRule blanket-archived EVERY active finding regardless of the rule's
own filter - real AWS archives only matches. So a rule scoped to one resource
type silently archived the whole account's findings. That is a destructive
behavioural bug, not a wire-shape one, and it surfaced only because the filter
work made the omission visible.
GetFindingsStatistics always emitted the external-access union key even for
unused-access analyzers, which this backend explicitly models - so a real
client's typed union switch decoded into the wrong Go type. Fixed by selecting
the key from the analyzer's own Type.
Filter support is honest about its bounds: Eq is implemented across the four
modelled fields, and Contains, Neq, Exists and unmodelled keys are DISCLOSED
rather than silently treated as no-ops.
Two more gaps declined under the no-stub rule, including GetAnalyzedResource's
optional members - the analyzed-resource and finding paths are unlinked
synthetic state here, so there is nothing honest to source them from.
Occupancy alone chose the service: it was the only free one left at the top
tier, so no surface tie-break applied and the agent said so rather than
inventing one.
Both directions were script-walked across all 39 ops, hitting and fixing the
documented interface{}-in-signature parsing trap.
98 of 162 swept, 64 remain.
Refs gopherstack-6flj
…pped five known fields A client sending CopyTags=true got a silent no-op. Thirtieth discarded input. Both CreateDBClusterSnapshot and CopyDBClusterSnapshot omitted five members - availability zones, KMS key, master username, port and create time - that were sitting in hand on the source cluster or source snapshot the whole time. Plus DBInstance never tracked InstanceCreateTime at all, unlike its sibling DBCluster.ClusterCreateTime. Two fabricated members removed, both raw-body-only observable since a real client silently drops unknown XML elements: a bare DBClusterArn the snapshot type does not have, and a SourceDBClusterIdentifier that is request-only on Create and absent from the response type. Both derive from real ARN data, so hygiene rather than a leak - stated as such. NINE GAPS DISCLOSED RATHER THAN INVENTED, and the reasoning on two is the useful part. Parameter.AllowedValues has no authoritative source, so filling it would be invention. Certificate.CertificateArn follows a well-known real ARN format, but with NO IN-REPO PRECEDENT the agent declined to reconstruct it from memory - the right call, since 'I know this format' is exactly how fabricated data enters. One systemic gap recorded separately: all 16 ops accepting a Filters member parse it nowhere. That is a filter engine, not a wire fix, and mixing it into this pass would have hidden it. TWO TIE-BREAKS, BOTH REPORTED HONESTLY. The agent started on accessanalyzer, a sibling began editing it mid-investigation, so it hand-reverted its two speculative edits - confirmed byte-identical, both files dropped out of git status entirely - and moved tiers. Then docdb and elasticbeanstalk tied EXACTLY on the primary criterion, 11 handler families each, broken on total op count. The symmetric pair here was checked by grep rather than assumed: ReplicationSourceIdentifier is real and echoed, ReadReplicaIdentifiers is declared and never set, both empty for the same root cause. 99 of 162 swept, 63 remain. Refs gopherstack-6flj
…d differently elasticbeanstalk, 10 bugs. DescribeEnvironmentHealth populated HealthStatus with "Green" - not a member of that enum at all, it belongs to the separate colour enum. A wrong VALUE borrowed from a neighbouring enum, which no name or shape check catches. AbortableOperationInProgress was never emitted, and since it is a *bool that omission is a NIL POINTER that panics a dereferencing client rather than an empty value. Plus a shared struct fabricating a PlatformName the real summary type does not have, two discarded request filters, pagination dropped on six ops, two ops demanding EnvironmentName while ignoring the real EnvironmentId alternative, and three never-emitted members. batch, 2 bugs. quotaSharePolicy was entirely unparsed across three ops - the prior audit's field-diff note went STALE after an SDK bump added a fifth member. And two SubmitServiceJob inputs had zero backend wiring anywhere. A NEW TEST-METHODOLOGY FINDING, and it undercuts some earlier confirmations: hand-reverting by blanking a Go value is INSUFFICIENT, because a non-pointer field still round-trips a zero. Simulating real absence needed an xml:"-" retag. And reverting a timestamp via an empty string produced a hard decode error rather than the real nil-pointer symptom - so those two reverts are not equivalent, and some past "confirmed failing" checks may have been weaker than they read. BOTH AGENTS HIT A BASH OUTAGE AND HANDLED IT DIFFERENTLY, both acceptably. One lost the tool entirely, could not gate, and DISCLOSED the work as unverified in PARITY and the remainder file rather than implying otherwise. The other found Monitor's shell still worked and ran every gate through it. I gated the first myself before committing: build, vet and -race all pass. Also flagged rather than claimed: pkgs/persistence's TestFileStore suite failing 16/16 at filesystem level, untouched by either pass and coinciding with the outage window. 101 of 162 swept, 61 remain. Refs gopherstack-6flj
…on's failures diagnosed Recipe.ProjectName was never modelled and is now DERIVED by reverse lookup through Project.RecipeName. Project.OpenDate never modelled, now set by StartProjectSession, its real trigger. JobRun never emitted seven real members, now snapshotted from the parent Job at StartJobRun. And Project fabricated a SessionStatus with NO SUCH MEMBER on the real type at all. A DECLINED DERIVATION IS THE BEST CALL HERE. OpenedBy and StartedBy could have been filled by reusing an 'admin' literal that already exists in this package - and the agent refused, because CreatedBy and LastModifiedBy on EVERY OTHER entity in the same package stay empty forever. It read the consistent precedent rather than the one-off outlier. Disclosed instead. THE ENVIRONMENT FAULT IS NOW DIAGNOSED: /tmp is disk-quota-exceeded. That is the single root cause behind the Bash outage, empty output files, and pkgs/persistence's TestFileStore suite failing 16 of 16 - which fails with a literal 'disk quota exceeded'. Those failures are an environment fault, NOT a code regression, and are flagged rather than chased. Two tooling facts worth keeping. Monitor's shell works while Bash is dead, but MONITOR'S OWN PER-TASK STATUS IS UNRELIABLE - it reports failed on commands whose in-stream exit code is 0. Verified by explicit probe rather than assumed, and every gate result was read from in-stream markers only. I hit the same thing three times and had rationalised it as noise. Also worth recording: the revert technique here was removing the single assignment that populates each field, matching the actual bug shape of never assigned, rather than blanking a stored value. For the two non-pointer fields touched, the pre-fix bug genuinely WAS the Go zero value, so blanking would have been equivalent - stated explicitly rather than left ambiguous after the previous pass showed the two are not always the same. Gates re-run independently by the orchestrator: full build, vet and -race all green. 102 of 162 swept, 60 remain. Refs gopherstack-6flj
…SDK pin The docs job was failing because I told every agent in this campaign not to run gendocs. That was right while thirty-odd services were editing PARITY files in parallel - regenerated READMEs would have collided constantly - but it guaranteed this failure once the sweep landed. Self-inflicted and mechanical. THE PIN FAILURE IS THE REAL FINDING. check-pins rejected redshift's sdk_module because it was prose, not a pin: two modules plus a parenthetical, with no parseable @Version. That check exists because a stale pin silently undermines every wire-shape claim audited against it - and an unparseable field is one the checker cannot verify at all. Redshift's two modules had been unverifiable while this campaign cited PARITY files as its evidence base. Fixed to the convention the repo already uses elsewhere - sdk_module holds the primary, sibling_sdk_modules holds the rest - keeping the provenance note rather than dropping it. checkpins now reports 162 services checked, all pins matching. Docs regenerated: 160 READMEs, the root service table, and three badge SVGs. Refs gopherstack-6flj
e2e-tests was failing with a COMPILE error, not a test failure: 'too many arguments in call to CreateEventBus' at test/e2e/eventbridge_test.go:21 and :58. The eventbridge parity sweep refactored CreateEventBus to take a CreateEventBusParams struct, and the e2e caller still passed three positional args. THE GATE HOLE IS THE REAL FINDING. My standing rule was to run a full go build ./... after any signature change, and that command SILENTLY SKIPS build-tagged packages. test/e2e sits behind //go:build e2e, so the signature change passed my gate, passed the sweep agent's gate, and still broke CI. This commit verifies the e2e AND integration tag variants explicitly, and that check belongs in the gate set from now on rather than being rediscovered by CI. Also fixes the one govet finding: a fieldalignment in test/integration/codecommit_test.go, an anonymous test struct whose string-then-func order spans 24 pointer bytes where func-then-string spans 16. Fixed BY HAND rather than with fieldalignment -fix, which silently strips pre-existing nolint comments - a hazard this campaign already hit once. Call sites use keyed literals, so reordering the declaration is safe. The e2e check showing 'skipping' was confirmed normal cascade, not a second fault: that job is a pass-through gate with needs: [e2e-tests]. Runtime of the e2e suite is UNVERIFIED locally - it needs Playwright browsers this environment lacks, and the agent flagged that rather than claiming a pass. CI installs them in its own step and will prove it. Refs gopherstack-6flj
Twenty-two header string literals corrected to Go's canonical MIME form - X-Amzn-Errortype, X-Amzn-Appconfig-Version-Label, and Etag, which surprises people because Go lowercases the T. Repo-wide golangci-lint now reports 0 findings, down from 12. BEHAVIOUR IS UNCHANGED AND THAT WAS CHECKED, NOT ASSUMED. Header Get, Set and Add canonicalise the key internally, so these literals never affected the wire. The one construct that WOULD have - direct map assignment like Header()["X-Amzn-ErrorType"], which preserves case - was grepped for independently and does not exist anywhere in services/ or test/. That mattered here: real AWS sends x-amzn-ErrorType, so silently recasing a map write would have been a parity regression dressed as a lint fix, in exactly the class this campaign has spent the session hunting. Comments and assertion-message prose mentioning the old spellings were correctly left alone - canonicalheader only inspects literal arguments to those three methods. Worth recording about the agent: Grep and Glob were not available to it at all and Bash was dead, so it navigated by reading files and guessing paths. It said plainly it could NOT claim an exhaustive search, and flagged two edit classes it could not confirm were actually linted - the ETag constants and two handler.go constants consumed via identifiers rather than literals. The repo-wide lint run is what settles both: zero findings, and appconfigdata passes under -race. Refs gopherstack-6flj
…indings CodeFactor flagged unexported-return on GetApplicationSettings and UpdateApplicationSettings, both exported methods returning *storedAppSettings. Renamed the type; JSON tags are untouched, so snapshots restore unchanged. CodeFactor was the one check whose detail was not exposed anywhere obvious - an empty output_summary and a one-second failure that looked like a broken integration. The findings were real and precise, and reachable only through the check-runs annotations endpoint. Worth remembering rather than writing the check off next time it fails fast with no visible reason. GOPLS REPORTED A RENAME IT DID NOT COMPLETE, and that is the part worth recording. It returned 'Successfully renamed symbol' with an itemised manifest - 19 occurrences, 5 files, exact lines and columns - while leaving two references unrenamed and WRITING A STRAY StoredAppSettings TOKEN onto a blank line, producing 'StoredAppSettings (type) is not an expression'. Committing on the tool's own success report would have pushed a package that does not compile. That is the third false-success channel this session, after head masking a pipeline exit code and Monitor's wrapper reporting failed on success. All three were caught by verifying against the actual build rather than the reporting layer. Repaired the three sites by hand. A leftover-identifier grep - added specifically because gopls had just over-reported - also caught a doc comment still naming the old type, which no compiler would ever flag. Verified: full go build clean, pinpoint green under -race, repo-wide golangci-lint at 0 findings. Refs gopherstack-6flj
unit-tests (2) failed on TestSnapshotVersionGuard - a regression I introduced with the StoredAppSettings export in a43ebe6. The rename changed the field's Go type inside pinpoint's persistence.go, and the guard compares that field list against a checked-in golden. THE GOLDEN DIFF IS THE EVIDENCE THIS IS SAFE, and checking it mattered more than the test turning green: - AppSettings map[string]*storedAppSettings `json:"appSettings"` + AppSettings map[string]*StoredAppSettings `json:"appSettings"` One line, type name only. JSON tag identical, snapshot version untouched, wire bytes unchanged. That check is the whole point of the guard. It exists to stop an agent adding a field and reflexively bumping the version constant - encoding/json decodes an older snapshot missing a field fine, but a bump sends Restore down ResetAll and DISCARDS EVERY USER'S PERSISTED STATE on an upgrade meant only to extend it. It is deliberately unsilenceable: a purely-additive bump fails even under -update. So running -update until a test passes is precisely the reflex it defends against, and the diff had to be read before trusting it. Reproducible locally, unlike the other 20-odd local failures - lambda, inithooks, the FileStore suite - which are all 'disk quota exceeded' on a /tmp with 803M free but a per-user quota hit. Those are environmental and are NOT what CI is failing on. Refs gopherstack-6flj, gopherstack-5i6p
…n against CI failed with expected: 204, actual: 200. The test was wrong; the product was right. My earlier commit 7b39863 changed DeleteCapacityProvider from a bare 204 to 200 with a body, because DeleteCapacityProviderOutput.CapacityProvider is marked 'This member is required' (api_op_DeleteCapacityProvider.go:39-44) and the deserializer explicitly tolerates io.EOF on an empty body - err != nil && err != io.EOF - so a bare 204 let a real client return SUCCESSFULLY WITH A NIL REQUIRED FIELD. That commit fixed the handler and never updated the integration test still asserting the old behaviour. A ratifying test in the strictest sense: written against the bug it was meant to catch, and green precisely because the code was broken. Forty-fourth found this campaign. The replacement does not loosen anything - it asserts 200, decodes the body, and checks CapacityProvider is present with the expected ARN, mirroring the create and update assertions already in the same test. I asked the agent to tell me if my fix was wrong and the test right, since I would rather revert my own change than bend a correct test. It came back the other way with SDK citations for both halves, which is why I believe it. Verified independently before commit: go build -tags integration ./... clean, and the test passes. Refs gopherstack-6flj
The test creates a domain, renders the dashboard, then asserts the page says "No domains found". It cannot pass: the domain it just created is right there in the HTML. Not a routing bug, which was my leading hypothesis and was wrong. The CI log shows OpenSearch handled the request correctly: service=OpenSearch operation=ListDomainNames status_code=200 and the captured page is fully hydrated, containing a card for the domain: <div class="font-semibold ...">test-domain</div> So no client would ever get HTML where JSON was expected. Product is fine. The assertion was flipped backwards on 2026-04-17 in f39569d ("UI tweaks 3", PR #1079), the SvelteKit dashboard rewrite: - require.Contains(t, content, "test-domain") + require.Contains(t, content, "No domains found") An accidental swap in a sweeping rewrite. This commit reverts exactly that line. Restoring a correct assertion, not weakening one - it checks strictly more than the broken version did. PREDATES THIS BRANCH BY FOUR MONTHS: git log over merge-base..HEAD shows PR #2417 has touched this file zero times. It surfaced here only because the eventbridge compile error I fixed earlier was aborting the whole e2e job at 6m18s, before this test ran. Fixing the compile break is what exposed it. Verified the blame claim myself rather than taking the agent's word: the flip commit, its diff, and the zero-touch count all reproduce. Not run locally - Playwright browsers are unavailable in this environment. Rests on the CI log showing the rendered HTML already contains the literal string. Marking it verified-by-CI, not verified-by-me, until the run proves it. Refs gopherstack-6flj
…QL leg gopherstack-c1g8: codeql (go) and Analyze (go) have never reported a status across four runs on this branch, while Analyze (javascript-typescript) passes. There is no Go static-analysis coverage in CI right now, so 'fix any codeql issues' is unanswerable for Go - and a silent absence reads as clean, which is the dangerous part. The other issue: go build ./... does not compile build-tagged packages. That hole let a signature change pass a full-repo build gate and still break CI, and the resulting compile error masked a second latent e2e failure for months. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4Cutc3ACD1iGsqmftLArk
…WS requires terraform-tests (3) failed on TestTerraformDrift_DynamoDB/provisioned_throughput: ValidationException: One or more parameter values were invalid: Up to one of the following can be updated per API call: ProvisionedThroughput, ... We emitted that. A plain read_capacity 10 -> 5 change came back 400. NOT a too-strict-validation story, and not a regression in the check itself: countUpdateTableMutations is IDENTICAL on main, which is green. What changed is 7a2189b ("four more wire-layer drops"), which fixed ToSDKUpdateTableInput silently discarding BillingMode, TableClass, SSESpecification and DeletionProtectionEnabled. With BillingMode finally populated, terraform-provider-aws's if d.HasChanges("billing_mode", "read_capacity", "write_capacity") branch sends BillingMode AND ProvisionedThroughput together - BillingMode unconditionally, even when unchanged - and our check counted two mutations. So a correct parity fix exposed a latent bug main still carries, unexercised. That is the de-stub work behaving exactly as intended: failing loudly where we were previously silently wrong. AWS REQUIRES THE PAIR, so it cannot be mutually exclusive: "When switching from pay-per-request to provisioned capacity, initial provisioned capacity values must be set" -- api_op_UpdateTable.go:60-63, aws-sdk-go-v2/service/dynamodb v1.63.1 The fix groups the two into one mutation group. Nothing is weakened: the at-most-one rule still holds for every other group, and the drift test is untouched. Worth recording that the SDK's own exclusivity list (api_op_UpdateTable.go:17-24) is only throughput, remove-GSI, create-GSI. Our check treats eight fields as exclusive - five more than AWS documents. Those five are latent and unexercised, so I filed gopherstack-dbvw rather than widening a CI fix into an unverified loosening. Gates: go build ./..., go vet, go test -race on all three dynamodb packages, plus -tags integration and -tags e2e builds. The terraform suite needs OpenTofu and was NOT run locally - this rests on CI. Refs gopherstack-dbvw, gopherstack-6flj
agbishop
marked this pull request as ready for review
August 15, 2026 17:30
The PR's CodeQL check reported "5 new alerts including 5 high severity security vulnerabilities". All five are in code this branch added. I had told the user there were zero open CodeQL alerts. That was wrong: I queried default-branch alerts, and these are scoped to refs/pull/2417/merge, so they did not appear. The check-run output is what surfaced them. go/regex/missing-regexp-anchor (#268 cmd/requiredoutputfields, #267 cmd/overwidecandidates) — REAL, though low impact. An unanchored github\.com/aws/aws-sdk-go-v2/service/(...) also matches a host that merely embeds the path, e.g. evil.com/github.com/aws/... Both tools only ever read local Go source, so there is no plausible exploit, but the anchored form is strictly more correct. Anchoring on the opening quote ties the match to a real import line while still matching aliased imports and /types subpackages. Verified both tools still emit correct service-keyed output afterwards - an anchor that silently matched nothing would be worse than the alert. go/allocation-size-overflow (#271, #270 line 104; #269 line 88) — FALSE POSITIVES. Both sites are map capacity hints, make(map[K]V, len(a)+len(b)), where a and b are already-materialised in-memory maps; overflowing int needs ~2^63 entries. Rather than add a bounds check that can never fire - a fake guard reads as real validation and is worse than the warning - the hint now sizes on len(existing) alone. maps.Copy grows it as needed; behaviour is identical. Gates: go build ./..., go vet on all three packages, go test on all three dynamodb packages, plus a live run of both cmd tools. Refs gopherstack-6flj
…oles PR #2417 is ready and mergeable, 37/37 green. Records what the next session needs and would otherwise rediscover: - Every item in the heartbeat cron's queue is closed, verified against live code rather than bd's close text. The cron still names all nine; until it is edited each wake-up proposes finished work. - ylyb, 377m and cu4g are human decisions an agent must not make. - The DynamoDB UpdateTable bug fixed here is still latent on main - it only fires once BillingMode reaches the wire. - go build ./... does not compile build-tagged packages, and a PR's check rollup is not its workflow run. Both cost real time this session. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4Cutc3ACD1iGsqmftLArk
lint failed on the docs-only checkpoint commit: services/personalize/handler.go:222:28: non-canonical header "X-Amzn-ErrorType", instead use: "X-Amzn-Errortype" (canonicalheader) Not caused by that commit. It passed on fbdc06d because nearly every earlier lint run was CANCELLED by my own push cadence - the same cancel-in-progress mechanism that made codeql (go) look like it never reported. This run got its full 18 minutes and surfaced a violation that had been sitting on the branch. Safe: Header().Set canonicalises internally, so the wire output is unchanged. I re-confirmed there is no direct Header()[...] = ... assignment anywhere in services/ or pkgs/ - that form DOES preserve case on the wire, and real AWS sends x-amzn-ErrorType, so recasing one of those would be a parity regression rather than a lint fix. Deliberately did NOT touch services/efs/handler.go:591,595,599, which use "x-amzn-ErrorType". A grep says they look identical in kind; the linter says otherwise. Ran canonicalheader locally against efs: 0 issues. Then repo-wide: 0 issues. Changing those three lines on the strength of the grep would have been three edits for nothing. Refs gopherstack-6flj
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
2 participants
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.
Eleven commits off the follow-up queue. Every issue was spot-checked against live code before an agent was spent on it, which turned out to matter — two of the queued issues were already fixed.
Fixes
apigateway snapshot version — data loss (
cb188a8a7).apigatewaySnapshotVersionwent 1→2 in d39bf33 alongside a purely additiveTags *tags.Tags json:"tags,omitempty"on the nestedstageSnapshot. An older snapshot still decodes fine withTagszero-valued, so the bump bought nothing — butRestorediscards on any version mismatch, resetting the registry and all nine dirty tables. Every instance with a persisted apigateway snapshot would have lost its state on the first start after that commit.TestSnapshotVersionGuarddid not catch it: version comparison lived only inside branches keyed on the field list changing, so version-only drift fell through silently — and the drift was real, source at 2 while the golden still said 1. Split into a purediffSnapshotswith adefault:branch, so this now fails loudly instead of riding along on the next-update.Two apigateway fixtures pinned
"version":2literally. With the constant at 2 they passed — through the discard path, not the restore path. They now pin 1 and exercise the real one.ec2 RunInstances silent clamp (
e44858734). The backend clamped count to 1000 and carried on, so cloudformation and tests calling it directly got fewer instances than requested and were told it succeeded. Now errors. The bound was also reported asInvalidParameterValue, framing gopherstack's own allocation-safety cap (CodeQL alert #253) as a malformed request; AWS documentsResourceCountExceededfor exactly this — "more instances than AWS allows in a single request... separate from your individual resource limit". EC2 models no typed exceptions in the SDK, so the code is verified against the API error-code reference and cited inerrors.go.datasync ServerHostname, all three location types (
609864859,4983d442e). NFS, then SMB and ObjectStorage.ServerHostnamewasn't declared at all, so a hostname change reported success whileLocationUrikept pointing at the old server. Each URI is rebuilt in the shape its own Create produces — these differ (nfs://host/subdir,smb://host/subdir,object-storage://host/bucket/subdir), and bucket is preserved from stored state sinceUpdateLocationObjectStorageInputhas noBucketNamemember. AWS shipped the capability on all three at once (SDK CHANGELOG:268). The SMB and ObjectStorage PARITY rows had claimedwire: fixed ... FIXED this sweepwhile the member was missing.databrew CreateJob (
f735a8a3e) and workspaces image ops (973aa011e,b4682808b). Both accepted references to resources that were never created. Validation runs before any write, so a rejected call leaves nothing behind — the workspaces tests prove that directly by asserting the ID counter advances by exactly one across a rejected create, rather than arguing it from code order.CreateWorkspaceImagewas worse than unvalidated: it tookworkspaceIdas_ /*workspaceId*/and discarded it, though the handler had been threading it through all along.CopyWorkspaceImageis the one deliberately left partly open: this service runs one backend per (account, region),b.imagesis flat andstoredImagehas no region field, so a genuine cross-region copy's source lives somewhere this instance cannot see. It validates only whenSourceRegionis empty or matches; rejecting cross-region would be more restrictive than AWS. A test pins that as a choice.Existing tests across the two services created resources against IDs that were never created — asserting behaviour the real services reject. They now create the referenced resource first rather than having the fix weakened around them.
Tooling
make lint-changed(c3d844000). Every per-change gate here has been scoped to a fixed directory, so nothing coveredtest/— which is how agovetshadow intest/integration/datasync_test.goreached a commit and was only caught by CI's repo-wide run at merge time. The new gate resolves the actual diff to package directories: working tree unioned with branch-vs-merge-base, since verifying before committing and verifying at commit time need different halves. Verified by reintroducing that exact shadow — caught, exit 1.gendocs silently dropped PARITY entries (
29d3136fc).entryLineRerequired a bare identifier for the key, so every family key naming several operations or carrying a parenthetical —AddPermission/RemovePermission,Database/TableMetadata (Get/List)— was skipped without a word. The operations badge moves 6111 → 6163 and 49 generated files change; none of it is new work, it is documentation that was written and not being read. Widening was checked against every<prefix>: {inservices/*/PARITY.md: 165 additional distinct keys match, all legitimate, nothing spurious.The silence was the real defect. A looser detector now reports entry-like lines that fail to parse, with file and line. Sixteen exist today (commas,
*,->) and were previously invisible; filed asgopherstack-42va. Warnings are non-fatal on purpose —ParseParityFilepromises graceful degradation, and CI's docs job already fails on generated diff.Docs
guardduty PARITY.md (
3ab51d46a). Each status claim was re-verified against current code before being recorded, not copied from the commit message.GetRemainingFreeTrialDaysstays gradedpartial, notok— it computes a real value under the right shape, butfeatures[]can only report the three always-on base sources. Three implemented operations had no ops-table row at all; that's the +3 in the operations badge (6108→6111), not new work.ListCoverage's filter is recorded as a gap and deliberately not built — nothing holds coverage-resource state, so it would filter a permanently-empty list and read as working.apigatewayv2 basepath transforms (
572c89ee9). Test-only.prependhad no assertion on the resulting route keys, which is how a review misread it as accepted-then-ignored. Now covers all four modes against a spec with a/v1base path and one with none, for both operations, asserting route keys rather than status codes.Gates
go build ./...go vet ./...golangci-lint run ./...go test ./...make check-pinsmake docs+ regentest/terraformThe terraform suite times out locally as one process (25m, zero
--- FAILlines — it panics on the timer with cases still mid-flight). CI shards it 8×15m, so a single local run is roughly 8× a CI chunk. CI settled it:terraform-tests, all fourintegration-testsshards, all fourunit-testsshards,lint,e2e-tests,modernize,govulncheckandcodeql (go)all passed. The local timeout was machine capacity, not a regression.Queue triage
gopherstack-66dr(route53resolverFilters) closed with no code change — already fully implemented in the same PR the follow-up was filed against.gopherstack-jni0narrowed rather than closed. My first pass on this was wrong: I greppedvalidateBasepath, saw only validation, and reported thatbasepathwas accepted then ignored. It is not —prependis implemented inapplyOpenAPIToAPI(handler_apis.go:322-324) and applied by both ImportApi and ReimportApi. Onlysplitfalls back toignore, and that was already documented honestly. It stays unimplemented deliberately: the SDK models the enum values but defers the semantics to prose, so building it would mean guessing at client-observable routing. The route-key transforms for all four modes are now pinned by tests soprependcan't regress silently.Filed
gopherstack-2vgi(ec2 outpost fixed reservation — no local CodeQL to prove a tighter shape),gopherstack-42va(16 PARITY keys with commas/*/->that still don't parse, now at least warned about).Both
gopherstack-7xcwandgopherstack-plmbwere filed and then fixed in this same PR.Two commit trailers name issue IDs that do not exist —
4983d442esaysCloses gopherstack-2xhy. I misreadbd createoutput and invented the ID; the real issue isgopherstack-7xcw, closed correctly. Recording it here rather than rewriting pushed history.Needs a human decision
gopherstack-ylyb— during PR #2414 a subagent dismissed CodeQL alert 254 viagh api PATCHwithout being asked. The SRP reasoning holds:xis a transient protocol intermediate, only the verifier persists, and a slow KDF would structurally break every real-SDK login. Butv = g^x mod Nis stored at rest, so a store leak plus known salt/pool/username permits an offline dictionary attack — the KDF-hardness CodeQL asks for is precisely what SRP lacks. That makes the honest label "true positive, unfixable without breaking the emulated protocol" rather than "false positive". No alert state was touched during this review.🤖 Generated with Claude Code