Skip to content

Make internal/sdkinv SDK-derived and provider-neutral - #21

Merged
rchildress87 merged 7 commits into
mainfrom
sdkinv-provider-neutral
Sep 29, 2026
Merged

rchildress87 merged 7 commits into
mainfrom
sdkinv-provider-neutral

Conversation

@rchildress87

Copy link
Copy Markdown
Contributor

Summary

internal/sdkinv derives the coverage denominator from each cloud's SDK, but it had drifted into hand-maintained word lists, provider code in the core, and SDK surface dropped without a trace. This PR removes all three so the subsystem scales to many providers: adding one touches no core file.

Six phases, one commit each, plus a separate docs pass:

  1. Core seams and an accounting invariant (4f1fca27). Universe.SourceOps / Dropped: every SDK op lands in exactly one of a candidate, Other, or Dropped with a reason, checked on fixtures and live caches. Scope vocabulary, Operation.Client, and Resolver.LabelOp replace provider special cases in the core. Denominator unchanged.
  2. Relocation (ce7f07b4). Each provider's extractor, refs, pairing resolver, pins and fixtures move to an SDK-free leaf internal/providers/<p>/<p>inventory, wired only through the slim-gated internal/providers/all/<p>.go. TestCoreIsProviderNeutral fails if a core identifier names a cloud. Denominator unchanged.
  3. AWS model-first (38ea8a13). Keys, listers and lineage come from Smithy resource shapes, items elements, paginator traits, httpLabel order and ARN grammar instead of verb and noun lists.
  4. GCP model-first (68df458f). Listers from pageToken/nextPageToken plus a sibling on the item schema; class from HTTP verb × flatPath; parents from matched get paths.
  5. Azure model-first (daa0d00c). A go/parser extractor over armX sources replaces the regex walk: builders are followed from every exported client method; the element is the decoded response field's shape; instance ids, namespace, scope and parent come from path structure. singletonIDs, armOwnModules, lowerFirst and the ref allow-lists are gone.
  6. Pairing cleanup (188e3446). Unaliased import names come from the importing file's selector roots, not a version-stripping guess; derived pairings require an exact service match. Live pairings byte-identical.

Then d9d1a68b audits every CLAUDE.md against the code (stale names, signatures, paths and numbers fixed; repeats collapsed into pointers).

Coverage impact

Provider main this PR
AWS 48.7% (1677/3441) 49.8% (1660/3336)
Azure 15.8% (398/2515) 16.1% (398/2478)
GCP 22.9% (239/1042) 23.2% (240/1034)

The denominators move because classification is now structural. Every changed row was classified in a live before/after diff; accepted misreads are recorded per provider in <p>inventory/CLAUDE.md ("Accepted residuals"). Zero unexplained types. docs/coverage.md and the baseline are regenerated.

User-visible changes

  • disco coverage prints a per-provider count of extractor warnings (--verbose lists them), e.g. an Azure arm module the binary links that the pinned SDK HEAD no longer holds.
  • Under -tags 'slim <p>', coverage sdk fetch|status knows only the compiled providers.
  • The Azure SDK cache KeepID changed, so the next make sdk-fetch refetches Azure.

Verification

  • CGO_ENABLED=0 go test ./... green; golangci-lint clean on changed packages; make check-coverage green.
  • Four hashed disco coverage services -o json runs agree (determinism).
  • Every structural rule mutation-checked against its fixture (break the rule, confirm a test goes red). One known gap: Azure b.paged || paged (fixture emits one builder per method).
  • Each phase closed with an adversarial review; every finding fixed or recorded.

Follow-ups (not in this PR)

  • store/scans.go:419,437, store/findings.go:148 order by rowid, which Postgres lacks — suspected, unverified.
  • --scan-as still documented in README.md, FEATURES.md and cmd/scans.go help; cmd/summary.go help shows snake_case by_account.
  • gcpinventory/extract.go:579 compares CanonSingular output instead of Ident.
  • cmd/disco-scaffold/gen.go scannerSigs per-provider table; AWS per-module version provenance; azure-rest-api-specs x-ms-arm-id as a second Azure ref source.

🤖 Generated with Claude Code

rchildress87 and others added 7 commits September 28, 2026 03:09
…ariant

Phase 1 of making internal/sdkinv purely SDK-derived and provider-neutral.
Coverage output (`coverage services -o json`, `coverage resolvers
--missing --with-refs -o json`) is byte-identical before and after.

- Universe.SourceOps / Dropped: every operation the sources declare,
  enumerated apart from classification, must land in a candidate's ops,
  Other, or Dropped with a reason. sdkinv.Unaccounted plus
  conformance.CheckUniverse enforce it on fixtures and live caches.
  GCP now reports the 6,591 ops it used to drop silently.
- Azure enumerates exported client methods independently of request
  builders; builders must be lowercase-first, which removes a phantom
  hdinsight op minted from an exported *CreateRequest method.
- Universe.Scopes: each extractor declares its scope vocabulary, narrowest
  first; coverage ranks Row.Scope by it instead of a cross-provider table.
- Operation.Client replaces splitting op names on "." in the pairing index.
- Resolver.LabelOp replaces the core's Azure "Client" strip in skew
  detection; (*Func).Line exported; pairing.Register panics on duplicates.
- ARM singleton ids move out of StripScopes: sdkinv.MarkIDs applies
  azure.SingletonIDs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rs/<p>/<p>inventory

Each provider's SDK inventory now lives in an SDK-free subpackage beside its
scanners: internal/providers/{aws/awsinventory,azure/azureinventory,
gcp/gcpinventory}. Each subpackage holds the extractor, refs, pairing resolver,
pins, scope vocabulary, fixtures, tests and its own CLAUDE.md. The core
(internal/sdkinv, pairing, conformance) keeps no provider names, pins or
vocabulary. TestCoreIsProviderNeutral tokenizes the core's non-test files and
fails when an identifier or string literal names a cloud or SDK format.

The inventory packages register from init() and are blank-imported by the
slim-gated internal/providers/all/<p>.go, so internal/sdkinv/all is gone. A
slim build now fetches and reports only the providers it links.
internal/providers/all derives its expected provider list from the scanner
registry. It also runs conformance.Check plus a pairing-resolver check over
every registered extractor (TestInventoriesConform), so a new provider cannot
skip either check.

Other changes:
- The pairing core tests run a fake provider end to end.
- Shared walk helpers move to pairing/pairingtest.
- azure_coverage.go uses azureinventory.IsSingletonID and
  LocationScopeSignal instead of its local duplicates.
- The CI SDK-cache key hashes the inventory packages.

Coverage output is byte-identical to the previous commit
(`coverage services` and `coverage resolvers --missing --with-refs`, run
from the repo root).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…word lists

Replace the verb/preposition/noun word lists in the AWS extractor with facts
the Smithy models and the Service Reference state: resource bindings and
identifiers, paginated/readonly/http/iamAction traits, and the catalog's
IsList/IsWrite/IsTaggingOnly annotations. Where neither source states a fact,
a structural rule decides and tags the candidate (Rule/Signals); the
surviving name rules are listed with their justification in
awsinventory/CLAUDE.md.

- model.go: service closure, resource tree (nestByIdentifiers,
  keepResourceSuffix), pagination merged over the service default.
- extract.go: the rule ladder (bindAction, lister rungs, cutQualifier,
  keyByShape, writtenBy/owns, isTag, lineage, classify).
- catalog.go: SR index, entry placement/admission ranking, assembly.
- resolver.go: OpKey from the SDK package name; LabelAliases include the
  model's own name.
- New gears fixture covers declared lineage, nesting, tagging and
  verb-prefix keys; ops unreachable from the service are Dropped.

AWS coverage moves from 48.74% to 49.76% (5399 candidates, 3336
resources); 20 scanned types fall to catalog/attribute and 10 are gained.
docs/coverage.md and the baseline are regenerated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…name lists

Rewrite the GCP extractor to read Discovery structure instead of method
names. An API is admitted when a document accepts the cloud-platform OAuth
scope. A lister is a GET on a collection path whose response element a
sibling confirms (item GET, item write or DELETE, create body, or a GET
answering an aggregated map element). Class comes from HTTP method x path:
collection create (or a create-by-verb), LRO shape or change handles,
created-elsewhere, delete-only, mutable (including compute action
sub-paths that answer the change handle), get-only, list-only. Scope pairs
strip only for declared roots and placements the document cannot create.
Same-collection routes fold within a document and, across documents, a
route through an unlisted container folds into the API's one collection of
that leaf and element (run v1 namespaces). Aggregated lists attach only to
same-scope twins. Parents resolve by item path. Refs walk inline schemas,
arrays and map values.

The resolver loses its import deny list and List/AggregatedList name set.
Op names compare canonically per segment, a bound *Service chain anchors
unconditionally, other bound SDK types only on known ops, and Other ops get
a scope-stripped label alias. The pairing core indexes every candidate an
op lists, and its label fallback resolves through Resolver.LabelOp. Core
gains SplitVerb.

Fixtures cover every rule; each of 22 rules was mutation-checked red.
Coverage regenerated: GCP 22.94% -> 23.21% (240/1034); AWS and Azure
unchanged. GCP now drops only non-cloud-api and alias-document ops.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…out spelling lists

Replace the regex walk over armX sources with a go/parser extractor. Builders are
any client method returning *policy.Request, followed transitively from every
exported method; the list element comes from the decoded response field's shape;
instance ids, the namespace, scope and parent come from path structure alone. The
singletonIDs, armOwnModules, lowerFirst and suffix/URI allow-lists are gone, and
refs are named by the serde wire names.

Review fixes: a single-model GET is promoted to lister for a written collection
nothing else lists (managementpartner partners, web hybrid connections and
premier add-ons); POST-only statics are actions, never ids; collections reached
only through locations/{l} are catalogs, not arm-envelope resources; a module
go/parser rejects is skipped with a warn diagnostic; disco coverage now prints
extractor warnings (count, or each under --verbose), including linked arm modules
absent at the pinned HEAD. TypeOwner binds the longest known client prefix.

Azure: 3395 candidates, 2478 resources (was 3524 / 2515); coverage 16.1%.
docs/coverage.md and the baseline are regenerated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…xact service

An unaliased import now binds the last path segment the importing file uses as a
selector root, instead of a version-stripping guess (google.golang.org/api/admin/
directory/v1 binds admin, not directory). The compiler rejects unused imports, so
the real name is always a qualifier in the file; no regex, no provider knowledge.

relatedTypes keeps only orphan types whose service segment equals the operation's
service. The prefix and candidate-leaf fallbacks never paired a live row; a type
spelling its service another way now stays unpaired and the gate reports it. The
fake-provider fixture gains a same-service derived row and a cross-service orphan.

Live pairings are byte-identical for aws, azure and gcp; coverage docs unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Verify each named function, file, flag, test and number against the tree and
fix what drifted: IAM now scans through GAAD (no scanIAMPolicies), the AWS
dispatch ladder gains Blocked and Deadline, region pre-scoping unions the SDK
endpoint table, ReportService takes a ServiceStatus, parallel scanning lives in
internal/scanrun, --scan-as and summary --by-account are gone, the Rego input
and finding shapes are camelCase, store's PK is a UUIDv7 with ResourceID in
root_id, and the AWS Service Reference pin is an index digest.

Collapse repeats into pointers to the owning file (ErrStoreWrite first rung,
EdgeDecl contract, add-a-type steps, redaction syntax), move the AWS account_id
resolution section from cmd to aws, and drop issue tags, commit hashes and
before/after narrative. Every invariant and footgun that still holds stays.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rchildress87
rchildress87 merged commit 69d16d0 into main Sep 29, 2026
2 checks passed
rchildress87 added a commit that referenced this pull request Oct 3, 2026
…, CLAUDE.md audit (#22)

* fix(store): make scan and check-run readers work on Postgres; widen --scan-id

Store correctness follow-ups from PR #21.

- Newest-first ordering: ListScans, LatestCompleteScan, ListCheckRuns
  tie-broke on SQLite's rowid, which does not exist on Postgres, so every
  one of them failed there. They and LatestIncompleteScan (which had no
  tiebreak) now order by newestFirst(): rowid on SQLite, keeping creation
  order for `latest`, and id on Postgres. Ids stay random hex because
  time-ordered ids would collide on the 8-char prefixes users paste.
- PersistCheckRun sent an unrebound `?` INSERT through tx.Exec, so
  `disco check --persist` failed on Postgres (42601). Findings tests now
  run under withDialects.
- `--scan-id` (resources, summary, tag-coverage) matches
  discovered_by OR verified_by, so a re-verify-only scan lists the rows
  `disco scans` counts instead of none. ResourceFilter.DiscoveredBy is
  renamed SeenBy. The removed --scan-as flag is gone from the docs;
  `disco diff` is the "new this run" view.
- Remove the checkpoint API, which never had a writer, and drop the
  scan_checkpoints table (migration 020, both dialects). --resume help now
  says what it does: reuse the scan id and re-list every service.
- Correct the ScanDiff comment and clear pflag Changed in resetGraphFlags
  (TestGraphBlast_PrincipalAutoFallback failed on every -count>1 run).

Release note: migration 020 raises the schema version, so read commands
against an existing database ask for `disco scan` to upgrade it, as with
any migration.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(cli): correct stale JSON keys and comments in help text

Help text and docs that still named keys from before the v0.18.0 camelCase
wire migration, so copied jq filters returned null:

- summary and `scans show` JSON envelope help, and their jq examples
- README summary and graph jq examples (`.native_id` printed an empty
  column for every row)
- snapshot help and verify messages/flag help naming manifest fields
  (tool_version, generated_at, scan_ids -> toolVersion, generatedAt, scans)
- diff jsonl comment (change_type -> changeType; the CSV header keeps
  change_type)

Also: `scans show` no longer claims the record carries the binary
version (nothing writes it); summary notes accountName is omitted when
unknown; the extractor-warning count agrees in number; the Diagnostic
severity comment lists what is emitted ("info" | "warn"); all.go names
the real per-provider files.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(sdkinv): key Azure modules by rp/armX; Ident scope compare; guard paged union

SDK inventory correctness follow-ups from PR #21.

- Azure: resources/armmanagedapplications and solutions/armmanagedapplications
  share a basename, so keyed by it one module's ops answered for the
  other's imports. ImportKey and OpKey now key by "<rp>/<armX>". Labels keep
  the "armX:" prefix; a new optional pairing.LabelModuler maps a key back,
  used by moduleAbsent, the only core site that compares a label's module
  prefix. Live effect: pairings, unpaired and `coverage services` output
  unchanged (both modules map to the same candidates); 14 sdk-skew messages
  now name the rp/armX key. Fixture gadgetry/armwidgets beside
  widgets/armwidgets pins the case where the candidates differ.
- GCP: a required query parameter names a tenancy root by Ident stem, so a
  plural ?projects= scopes to project. Live output unchanged; fixture
  widgets/shelves pins it.
- Azure: fixture method Refresh reaches the list builder after the pager,
  guarding `b.paged = b.paged || paged`, which no test covered.
- sdk-skew message no longer doubles the module; docs record the key
  scheme, the optional resolver hooks and why idx.other stays
  single-valued.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(coverage): providers own their scaffold stub; warn on AWS modules without a model

- The stub scanner disco-scaffold emits (imports, serviceEntry fn
  signature, TODO body) moved from a provider-keyed table in
  cmd/disco-scaffold to an optional coverage.ScaffoldStubber each
  provider's coverageProvider implements. Scaffold output is
  byte-identical for every provider, registered or not. Tests now fail if
  a registered provider drops its stub, or if a stub's signature drifts
  from that provider's serviceEntry.fn type (which used to yield a
  scaffold that parsed but did not compile).
- AWS absentModels warns for each linked aws-sdk-go-v2/service module
  whose package has no Smithy model at SDKRef, the analogue of Azure's
  absentModules. Existence only; a per-module version compare would warn
  whenever go.mod lags the pin. Live: 0 of 312 linked service packages
  absent; coverage output unchanged. linkedModules moved to
  sdkinv.LinkedModules for both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(azure): record the x-ms-arm-id refs spike and defer it

Measured whether azure-rest-api-specs arm-id annotations would add refs
the shape rule misses. Joined at each SDK client's api-version, with
common-types resolved, the element's own id skipped and discriminator
subtypes walked: 127 new within-depth refs on 64 of 2,478 resource
candidates (2.6%), while 88% of today's shape refs carry no annotation.
Not worth a 64-214 MB source plus per-module version selection; the
numbers, the join requirements and the corrected first measurement are
in azureinventory/CLAUDE.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: audit and tighten every CLAUDE.md; regroup the AWS guide

claude-md-improver pass over the repo's CLAUDE.md files, with each
claim checked against the code.

Wrong facts corrected:
- store: placeholders do carry a marker (reference_only, 011); the
  InsertResourcesIfAbsent caller list missed three GCP resolvers
  (now a grep pointer); check-migrations is not run in CI;
  resource_count is computed in SQL (scanResourceCountExpr), not
  passed in; UpsertQuotas is a withWriteRetry writer; dimension_key
  came in 019.
- cmd: the EventBridge wrapper key is "Rule", not "rule" (a Rego rule
  copied from the doc matched nothing); service-filter examples come
  from each provider's ServiceFilterExemplar; incomplete --output lists.
- root: the linter formats with gofumpt + goimports; there is no
  project gofmt config. Use golangci-lint fmt.
- Leftovers from this branch: the Azure paged-union rule is now
  covered, x-ms-arm-id is deferred rather than "the follow-up",
  ScaffoldStubber was missing from coverage and the sdkinv README, the
  GCP scope rule now compares Ident stems, and absentModels and
  absentModules were attributed to the wrong files.
- providers: three design rules, not two; gcp's pairing guard also
  accepts explained unpaired reasons; sdkinv README's --with-refs row.

Code comments: pairing.Diagnostic.Kind listed a kind nothing emits
and omitted sdk-skew/sdk-module-absent; store/dialect.go's
placeholder() doc comment sat on newestFirst().

Duplication: each fact now has one owner (store owns newestFirst,
SeenBy, the JSON contract and the ErrStoreWrite rationale; providers
owns slim wiring and redaction rules; inventory files own their drop
counts and live sizes), with pointers elsewhere. Incident narratives
and frozen counts were cut; the x-ms-arm-id spike keeps its decision
numbers (full numbers in 3b16282).

internal/providers/aws/CLAUDE.md: ~75 flat sections regrouped under
seven parent headings. Every original body line is kept verbatim except one
cross-reference reworded to name its target section; one
pair of sections merged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: move release-pipeline prose to docs/release.md; RLS rule to the Postgres section

The root CLAUDE.md carried ~25 lines of release-engineering narrative
(SBOM generation, the govulncheck gate, binary vs source mode). The
full explanation now lives in docs/release.md. The root keeps only
the rules an edit to the Makefile, release.yaml or go.mod must not
break: SBOMs from the raw binary before compression, pinned tool
versions kept in sync, never vulncheck a stripped binary, and bump
the Dockerfile GO_VERSION with the go directive.

store/CLAUDE.md: the row-level-security requirements for an embedder
widening the relationships key moved from the Tables bullet to the
Postgres backend section, as a numbered list. The text maps each
missed condition to its failure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant