Fix effective SDK platform availability in OpenAPI specs - #9
anurag6569201 wants to merge 1 commit into
Conversation
Source PR: appwrite#13502 Source head: bb45fed
✅ Shipwright · ApproveRecommendation: approve PR #9 · Tier
Findings (5)
Fireworks usage: 14,656 input · 663 output · 15,319 total tokens · $0.0037 · 13s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
|
|
||
| public static array $errors = []; | ||
|
|
||
| /** @var list<string>|null Null derives membership from auth; an explicit empty list stays empty. */ |
There was a problem hiding this comment.
Shipwright · HIGH
The comment 'Null derives membership from auth; an explicit empty list stays empty' describes a subtle distinction that is not enforced by the type system.
Impact: The comment 'Null derives membership from auth; an explicit empty list stays empty' describes a subtle distinction that is not enforced by the type system. A future caller cannot tell from the API whether setPlatforms([]) means 'no platforms' or 'not yet resolved', and getPlatforms() silently falls back to auth-derived platforms only when the property is null. This implicit state machine is easy to misuse.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| throw new Exception('Failed to create specs directory: ' . $specsDir); | ||
| } | ||
|
|
||
| // Resolve full auth arrays through the active task (including subclass platforms). |
There was a problem hiding this comment.
Shipwright · HIGH
The new pre-resolution loop in Specs::action() calls setPlatforms() on every SDK Method before the per-platform loop, but getPlatforms() can still return an empty list when a Metho
Impact: The new pre-resolution loop in Specs::action() calls setPlatforms() on every SDK Method before the per-platform loop, but getPlatforms() can still return an empty list when a Method has no namespace or is hidden. The per-platform loop then skips the route, silently dropping it from all generated specs. If any route relies on the old behavior where namespace/hide checks happened later or differently, that route disap…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| */ | ||
| case ORGANIZATION = APP_AUTH_TYPE_ORGANIZATION; | ||
|
|
||
| public function getPlatform(): ?string |
There was a problem hiding this comment.
Shipwright · HIGH
getPlatforms() derives platforms from getAuth() when $this->platforms is null, but AuthType::ORGANIZATION maps to null and is filtered out.
Impact: getPlatforms() derives platforms from getAuth() when $this->platforms is null, but AuthType::ORGANIZATION maps to null and is filtered out. A route secured only by ORGANIZATION will produce an empty platform list and be excluded from every SDK spec, even though the old switch also had no case for ORGANIZATION and would have produced an empty list. This is a latent behavior change risk if ORGANIZATION-only routes…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| continue; | ||
| } | ||
|
|
||
| $sdkPlatforms = []; |
There was a problem hiding this comment.
Shipwright · HIGH
The OpenAPI3 parser now trusts Method::getPlatforms() to decide whether a route is emitted for a platform.
Impact: The OpenAPI3 parser now trusts Method::getPlatforms() to decide whether a route is emitted for a platform. Because getPlatforms() can be populated by the active specs producer via setPlatforms(), any producer that fails to call setPlatforms() or calls it with incorrect data will cause routes to be omitted from generated API specs. This is a spec-generation integrity risk rather than a runtime auth bypass, but it can…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| throw new Exception('Failed to create specs directory: ' . $specsDir); | ||
| } | ||
|
|
||
| // Resolve full auth arrays through the active task (including subclass platforms). |
There was a problem hiding this comment.
Shipwright · LOW
The new pre-resolution loop mutates SDK Method objects in place while iterating over $appRoutes.
Impact: The new pre-resolution loop mutates SDK Method objects in place while iterating over $appRoutes. This side effect is not obvious from the surrounding code and makes the action() method harder to reason about because platform resolution now happens in two places: the pre-loop and getPlatforms() fallback.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
What does this PR do?
Fixes
x-appwrite.platformsto match actual SDK availability:serverfrom OAuth2 session creation and usage events/gauges, where it is hidden.serverplatform to presence update/upsert.platformsto individual method aliases so their availability is explicit.This does not consolidate the three specs or change their filenames. Existing generated SDK output is unchanged.
Spec changes
Excerpts below omit unrelated fields.
OAuth2 session creation
GET /account/sessions/oauth2/{provider}already excludes server SDKs; its metadata now agrees:"x-appwrite": { - "platforms": ["console", "client", "server"] + "platforms": ["console", "client"] }Usage events/gauges similarly change from
["console", "server"]to["console"].Presence update — server spec
"operationId": "presencesUpdate", "x-appwrite": { - "platforms": ["client", "console"], + "platforms": ["client", "console", "server"], "methods": [ { "name": "update", "namespace": "presences", + "platforms": ["server"], "required": ["presenceId", "userId"] } ] }Presence update — client spec
"operationId": "presencesUpdate", "x-appwrite": { - "platforms": ["client", "console"], + "platforms": ["client", "console", "server"], "methods": [ { "name": "update", "namespace": "presences", + "platforms": ["client", "console"], "required": ["presenceId"] } ] }The
userIdrequirements are unchanged. The new metadata identifies the existing variants.Validation
Related to CLO-4377.
Source merge-base:
4176480f5d8bd88fc831575ed277bfb66abff894Source head:
bb45feda792f8e2631d6cd01d862c8526893af4c