Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .last-synced-sha
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7cf31eec6b8b4bc94ad975aabddc5b61da65c73c
53bf6960607cb91b93bc11a86901bdf68e90bb30
35 changes: 34 additions & 1 deletion scripts/__tests__/sdk-release-metadata.spec.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import assert from 'node:assert/strict';
import test from 'node:test';

import { factsFromDiff, renderChangelogMarkdown, scopesForServices } from '../sdk-release-metadata.mjs';
import { factsFromCompat, factsFromDiff, renderChangelogMarkdown, scopesForServices } from '../sdk-release-metadata.mjs';

// factsFromDiff only reaches indexes.symbolScopes (scope resolution) for the
// kinds under test; an empty index leaves scope unresolved, which is fine β€” we
Expand Down Expand Up @@ -161,3 +161,36 @@ test('renderChangelogMarkdown keeps distinct scopes as separate headings', () =>
assert.ok(markdown.includes('**[sso](https://s)**:'));
assert.ok(markdown.indexOf('[pipes]') < markdown.indexOf('[sso]'));
});

// Regression: a compat surface change must resolve to a real scope with a
// docs_url, never the `sdk` fallback (which hard-fails --strict-scopes in CI).
// factsFromCompat distrusts a service scope that is merely `toSnakeCase(root)`
// and defers to scopeFromName; every root below reached CI as `sdk` because
// scopeFromName had no rule for it. These map one-per-scope (the per-scope
// dedup keeps a single breaking fact each) so all four resolve in one pass.
const compatSurfaceScopeCases = [
{ symbol: 'AdminPortal.generate_link', scope: 'admin_portal' },
{ symbol: 'Authorization.assign_role', scope: 'authorization' },
{ symbol: 'Agents.create_validate', scope: 'agents' },
{ symbol: 'ClientApi.create_token', scope: 'client' },
{ symbol: 'CreateApplicationSecret.from_dict', scope: 'connect' },
];

for (const { symbol, scope } of compatSurfaceScopeCases) {
test(`factsFromCompat resolves ${symbol} to ${scope}, not sdk`, () => {
const compatReport = {
changes: [
{
severity: 'breaking',
category: 'parameter_type_narrowed',
symbol,
message: `Parameter type changed for "x" on "${symbol}"`,
},
],
};
const facts = factsFromCompat(compatReport, [], EMPTY_INDEXES);
assert.equal(facts.length, 1);
assert.equal(facts[0].scope, scope);
assert.notEqual(facts[0].scope, 'sdk');
});
}
60 changes: 59 additions & 1 deletion scripts/sdk-compat-pr-comment.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,59 @@ function renderMarkdown(languageData, options) {
return lines.join('\n') + '\n';
}

// GitHub rejects issue comments whose body exceeds this many characters
// (HTTP 422 "Body is too long"). Leave headroom below the hard limit for the
// truncation footer and any <details> tags we close on the way out.
const GITHUB_COMMENT_LIMIT = 65536;

/**
* Cap the comment body to GitHub's issue-comment size limit.
*
* The full report can balloon on large spec diffs (every affected symbol in
* every language). When it overflows we trim at a line boundary, close any
* <details> blocks we cut through so the HTML stays balanced, and append a
* note pointing at the complete report (Pages URL or the run's artifact).
*/
function truncateForComment(markdown, { pagesUrl, runId, repo }) {
if (markdown.length <= GITHUB_COMMENT_LIMIT) return markdown;

const linkTarget = pagesUrl
? `[the full SDK diff report](${pagesUrl})`
: runId && repo
? `the \`sdk-diff-report.html\` artifact on [this run](https://github.com/${repo}/actions/runs/${runId}#artifacts)`
: 'the full SDK diff report';

const footer = [
'',
'> [!NOTE]',
`> This report was truncated because it exceeded GitHub's ${GITHUB_COMMENT_LIMIT.toLocaleString('en-US')}-character comment limit. See ${linkTarget} for the complete details.`,
'',
].join('\n');

// Reserve room for the footer plus a handful of closing </details> tags.
const budget = GITHUB_COMMENT_LIMIT - footer.length - 256;

const kept = [];
let length = 0;
let openDetails = 0;
for (const line of markdown.split('\n')) {
const lineCost = line.length + 1; // account for the rejoined newline
if (length + lineCost > budget) break;
kept.push(line);
length += lineCost;
openDetails += (line.match(/<details[ >]/g) ?? []).length;
openDetails -= (line.match(/<\/details>/g) ?? []).length;
}

// Balance any <details> blocks the cut left open.
while (openDetails > 0) {
kept.push('</details>');
openDetails -= 1;
}

return kept.join('\n') + footer;
}

function main() {
const args = parseArgs(process.argv);
const artifactDirs = listArtifactDirs(args.artifactsRoot);
Expand All @@ -1147,7 +1200,12 @@ function main() {
codeDiffAvailable: args.codeDiffAvailable,
pagesUrl: args.pagesUrl,
});
fs.writeFileSync(args.output, markdown, 'utf8');
const capped = truncateForComment(markdown, {
pagesUrl: args.pagesUrl,
runId: args.runId,
repo: args.repo,
});
fs.writeFileSync(args.output, capped, 'utf8');
}

main();
23 changes: 19 additions & 4 deletions scripts/sdk-release-metadata.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { pathToFileURL } from 'node:url';

const SCOPE_LABELS = {
admin_portal: 'admin portal',
agents: 'agents',
api_keys: 'API key',
audit_logs: 'audit log',
authorization: 'authorization',
Expand Down Expand Up @@ -36,6 +37,7 @@ const SCOPE_LABELS = {

const SCOPE_DOC_URLS = {
admin_portal: 'https://workos.com/docs/reference/admin-portal',
agents: 'https://workos.com/docs/reference/agents',
api_keys: 'https://workos.com/docs/reference/authkit/api-keys',
audit_logs: 'https://workos.com/docs/reference/audit-logs',
authorization: 'https://workos.com/docs/reference/fga',
Expand Down Expand Up @@ -412,6 +414,15 @@ function scopeFromName(name) {
if (/^Radar/.test(name)) return 'radar';
if (/^(Vault|Object$|ObjectMetadata|ObjectSummary|ObjectVersion|ObjectWithoutValue)/.test(name)) return 'vault';
if (/^AuditLog/.test(name)) return 'audit_logs';
// `/agents/*` standalone surface (`Agents.create_validate`, `.get_registration`,
// `.update_attempts`) and agent-registration types.
if (/^Agent/.test(name)) return 'agents';
// `/client/token` issuance (`ClientApi.create_token`). Distinct root from the
// bare `Client` the compat client rule already maps.
if (/^ClientApi/.test(name)) return 'client';
// Connect application secrets (`CreateApplicationSecret`, `NewConnectApplicationSecret`);
// the `Create`/`New` prefixes dodge the anchored `Application|Connect` rule below.
if (/ApplicationSecret/.test(name)) return 'connect';
if (/^(Application|Connect|UserObject$|ApplicationCredentials|ExternalAuth|RedirectUriInput)/.test(name)) return 'connect';
if (/^(Group|CreateGroup|UpdateGroup)/.test(name)) return 'groups';
if (/OrganizationMembership/.test(name)) return 'organization_membership';
Expand All @@ -426,13 +437,17 @@ function scopeFromName(name) {
if (/^(Invitation|MagicAuth|PasswordReset|RevokeSession|Session|User|CreateUser|UpdateUser|EmailChange)/.test(name)) {
return 'user_management';
}
if (/^(Role|Permission)/.test(name)) return 'authorization';
if (/^(Role|Permission|Authorization)/.test(name)) return 'authorization';
if (/^Widget/.test(name)) return 'widgets';
if (/^Event/.test(name)) return 'events';
// Admin Portal generate-link intent options (`GenerateLinkDto`). The SSO and
// Admin Portal service surface (`AdminPortal.generate_link`) and its
// generate-link intent options (`GenerateLinkDto`). The service scope equals
// `toSnakeCase('AdminPortal')` (`admin_portal`), so factsFromCompat falls back
// to this name rule; without it the surface resolves to `sdk`. The SSO and
// domain-verification variants resolve to their own scopes via the rules
// above; this catches the bare `IntentOptions` aggregate and any other
// above; `IntentOptions$` catches the bare aggregate and any other
// intent-options type not otherwise classified.
if (/^AdminPortal/.test(name)) return 'admin_portal';
if (/IntentOptions$/.test(name)) return 'admin_portal';

return 'sdk';
Expand Down Expand Up @@ -938,7 +953,7 @@ function compatChangeIsBreaking(change, indexes) {
return true;
}

function factsFromCompat(compatReport, existingFacts, indexes) {
export function factsFromCompat(compatReport, existingFacts, indexes) {
const facts = [];
const existingBreakingScopes = new Set(existingFacts.filter((fact) => fact.severity === 'breaking').map((fact) => fact.scope));
const renames = renamesFromCompat(compatReport);
Expand Down
43 changes: 29 additions & 14 deletions spec/open-api-spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13122,20 +13122,35 @@ paths:
content:
application/json:
schema:
type: object
properties:
code:
type: string
description: The error code identifying the type of error.
example: daily_quota_exceeded
const: daily_quota_exceeded
message:
type: string
description: A human-readable description of the error.
example: Request could not be processed.
required:
- code
- message
oneOf:
- type: object
properties:
code:
type: string
description: The error code identifying the type of error.
example: daily_quota_exceeded
const: daily_quota_exceeded
message:
type: string
description: A human-readable description of the error.
example: Request could not be processed.
required:
- code
- message
- type: object
properties:
error:
type: string
description: The OAuth error code.
example: too_many_requests
const: too_many_requests
error_description:
type: string
description: A human-readable description of the error.
example: 'The request failed due to: too_many_requests.'
required:
- error
- error_description
Comment on lines +13125 to +13153

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 oneOf without a discriminator makes variant selection ambiguous for tooling

The two oneOf variants share no common property name (code/message vs error/error_description), so an OpenAPI discriminator cannot be applied. Code generators and strictly-typed SDK consumers will produce union types with no programmatic way to distinguish which variant was received β€” they must fall back to inspecting field presence at runtime. Consider documenting the dispatch rule (e.g., OAuth-grant requests receive the error/error_description shape; all other requests receive code/message) in the description field of the 429 response (which is currently empty) so that consumers understand when to expect each variant.

Prompt To Fix With AI
This is a comment left during a code review.
Path: spec/open-api-spec.yaml
Line: 13125-13153

Comment:
**`oneOf` without a discriminator makes variant selection ambiguous for tooling**

The two `oneOf` variants share no common property name (`code`/`message` vs `error`/`error_description`), so an OpenAPI `discriminator` cannot be applied. Code generators and strictly-typed SDK consumers will produce union types with no programmatic way to distinguish which variant was received β€” they must fall back to inspecting field presence at runtime. Consider documenting the dispatch rule (e.g., OAuth-grant requests receive the `error`/`error_description` shape; all other requests receive `code`/`message`) in the `description` field of the `429` response (which is currently empty) so that consumers understand when to expect each variant.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

summary: Authenticate
tags:
- user-management.authentication
Expand Down
Loading