docs: publish the v1.0 compatibility policy, and lock the surfaces it promises - #129
docs: publish the v1.0 compatibility policy, and lock the surfaces it promises#129skyoo2003 wants to merge 1 commit into
Conversation
… promises The only stability policy in the repo guaranteed `ServicePlugin`, `PluginConfig`, `Response`, `Resource` and the `ProtocolType` constants across v1.x. All five live in `internal/plugin`, which Go forbids importing from another module — so the sole documented v1.0 guarantee applied to nobody. docs/compatibility-policy.md states the promise for the surfaces a user is actually exposed to: config keys, environment variables, the CLI, admin API response keys, fidelity tier names, and wire behaviour. The wire guarantee is scoped to hand-verified operations covered by tests/compatibility/ — that suite, 775 tests run in CI and again against the tagged commit, is the enforcement rather than prose. Everything else is listed as explicitly not guaranteed, including auto-crud response content and all of `internal/`, which keeps the planned IR and ModelSource refactor unblocked. Two surface-lock tests keep the document honest. The admin ones matter most: the existing tests decode responses into the internal structs, so renaming a JSON tag renames both sides of the assertion and they stay green while every consumer breaks. Decoding into map[string]any and asserting key presence catches it — verified by renaming `resourceCount` and watching only the new test fail. Asserting presence rather than payload equality keeps the additive change the policy permits from failing the build. Also scopes the plugin-api.md guarantee to in-tree contributors, cross-links the policy from the release pre-flight checklist, and corrects the docs index, which still claimed 101 services.
github-actions
Bot
added
documentation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64908017f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | Key | Type | Meaning | | ||
| |---|---|---| | ||
| | `server.port` | int | Listen port. Default `4747` when absent or `0`. | | ||
| | `services` | map | Presence of the block is authoritative — only the services it lists run. Absent means every registered service runs. | |
There was a problem hiding this comment.
For a config containing services: {}, this promise is false: YAML creates an empty map, but Config.Service checks len(c.Services) > 0, so it treats that explicitly present block exactly like an absent block and enables every registered service. This can unexpectedly start all 104 services when a user relies on the newly published authoritative-block guarantee; distinguish a nil map from an empty non-nil map, or narrow the policy.
Useful? React with 👍 / 👎.
| | `DEVCLOUD_SERVICES` | Service filter. `all`, a comma-separated list of service ids, or the `tier1` / `tier2` / `tier3` shortcuts. Unknown tokens are treated as literal service names. | | ||
| | `DEVCLOUD_DATA_DIR` | Base directory; each service stores under `<base>/<id>`. Overrides `data_dir`. | | ||
|
|
||
| Environment overrides config file, and that precedence is guaranteed. |
There was a problem hiding this comment.
This precedence guarantee does not hold for DEVCLOUD_SERVICES: with YAML listing only s3 and DEVCLOUD_SERVICES=sqs, Config.Service("sqs") first accepts the environment filter but then rejects sqs because the nonempty YAML map does not list it, leaving no service enabled. The added test masks this case by selecting s3, which is already present in YAML; either make the environment selection authoritative or document that it can only further restrict the YAML block.
Useful? React with 👍 / 👎.
| **A hand-verified operation covered by a test in [`tests/compatibility/`](../tests/compatibility/) | ||
| keeps its response shape across 1.x.** | ||
|
|
||
| That suite — 775 tests driving real boto3 clients — *is* the guarantee. It runs in CI on every | ||
| push and again against the tagged commit before a release publishes, so the promise is enforced | ||
| by a failing build rather than by review discipline. If a response shape you depend on is not |
There was a problem hiding this comment.
The compatibility suite does not currently enforce an operation's full response shape. For example, tests/compatibility/test_lambda.py:27-38 covers the hand-verified CreateFunction operation but asserts only FunctionName and FunctionArn; removing fields such as Runtime, Handler, or MemorySize from functionConfig would still leave that test green while breaking users under this newly stated guarantee. The policy must scope stability to fields explicitly asserted by tests, or the suite must snapshot/validate every promised response field before claiming a failing build enforces the guarantee.
Useful? React with 👍 / 👎.
| - **Error message wording.** Error *codes* and HTTP status of `unimplemented` operations are | ||
| documented in [fidelity-manifest.md](fidelity-manifest.md); the human-readable message text | ||
| is not stable. |
There was a problem hiding this comment.
This newly guarantees error codes and statuses that the linked fidelity manifest describes incorrectly for many classified operations. For example, IAM classifies AcceptDelegationRequest as unimplemented, but the default branch in internal/services/iam/provider.go returns NotImplemented with HTTP 501, while docs/fidelity-manifest.md:17 says Query services return InvalidAction with HTTP 400; numerous JSON providers have the same 501 behavior. Publishing those values as stable therefore gives v1 users a contract the current binary already violates, so either normalize the providers or document their actual per-service errors before guaranteeing them.
Useful? React with 👍 / 👎.
| - **Everything under `internal/`.** Go forbids importing it from another module, and DevCloud | ||
| reserves the right to restructure it freely across 1.x — explicitly including the planned | ||
| intermediate representation and `ModelSource` work on the [roadmap](roadmap.md). Internal | ||
| churn is not a compatibility event. |
There was a problem hiding this comment.
The blanket statement that everything under internal/ may be restructured freely across 1.x directly conflicts with the compatibility contract added to docs/plugin-api.md:130-142, which says the in-tree ServicePlugin, PluginConfig, Response, Resource, and ProtocolType surfaces are stable and require a major bump for breaking changes. Maintainers therefore cannot tell whether changing those types is permitted in a 1.x release; carve that explicitly stable subset out of this exclusion or remove the competing contract.
Useful? React with 👍 / 👎.
1 participant
Summary
The only stability policy in the repo guarantees
ServicePlugin,PluginConfig,Response,Resourceand theProtocolTypeconstants across v1.x — all five live ininternal/plugin, which Go forbids importing from another module, so the sole documented v1.0 guarantee applies to nobody. This PR writes the promise for the surfaces a user is actually exposed to, and adds tests that fail the build when one of them drifts.Milestone 2 of the AWS v1 stabilization work. With this, the only thing left before tagging v1.0 is the tag itself.
Related Issue
Refs #129 — no prior issue; the changelog fragment uses this PR's number, per the convention in #126–#128.
Changes
docs/compatibility-policy.md(new) — what v1.0 guarantees across 1.x, per surface: config keys, environment variables, the CLI, admin API response keys, fidelity tier names, and wire behaviour. Plus what is explicitly not guaranteed, and the deprecation procedure that must precede any removal.tests/compatibility/. That suite — 775 tests, run in CI and again against the tagged commit before a release publishes — is the enforcement, rather than prose backed by review discipline.auto-crudresponse content (948 operations), hand-verified operations with no compat test, store durability, and all ofinternal/are listed as not guaranteed. The last one keeps the planned IR /ModelSourcerefactor unblocked.internal/config/config_test.go—TestGuaranteedConfigSurfaceandTestGuaranteedEnvSurfaceassert every guaranteed key and env var lands on the expected field.internal/admin/api_test.go—TestGuaranteedAdminSurface_{Collections,Fidelity}assert the wire keys of all four routes.map[string]anyon purpose. The existing tests decode into the internal structs (serviceInfo,RequestLog), so renaming a JSON tag renames both sides of the assertion and they stay green while every consumer breaks.docs/plugin-api.md— scopes its existing guarantee to in-tree contributors and points users at the new policy.docs/release.md— resolves the cross-link the pre-flight checklist from ci: enforce the checks a v1.0 tag depends on #127 was written expecting; adds a compatibility-review step.README.md/docs/README.md— index the new doc; corrects the docs index, which still claimed 101 services.Test Plan
CGO_ENABLED=0 go test ./...— all packages pass.golangci-lint run ./internal/config/... ./internal/admin/...— 0 issues.json:"resourceCount"→json:"resource_count"ininternal/admin/api.go→TestGuaranteedAdminSurface_Collectionsfails naming the missing key.TestAPI_Servicesstays green, which is exactly the gap this closes.yaml:"format"→yaml:"fmt"ininternal/config/config.go→TestGuaranteedConfigSurfacefails onlogging.format.changie batch v9.9.9 --dry-run— 29 entries, 0 dead issue links, so the release gate atrelease.yml:225passes.Checklist
golangci-lint run)