fix: rename binding to createOctokit and harden option merging

- Rename context binding from getOctokit to createOctokit to avoid
  SyntaxError when users write const { getOctokit } = require(...)
  in their scripts (~10 public workflows affected)
- Strip undefined values from user options to prevent clobbering
  defaults (e.g. GHES baseUrl)
- Deep-merge retry options alongside request options
- Use nullish coalescing (??) instead of logical OR (||)
- Shallow-copy opts to prevent shared reference mutation
- Add tests: undefined stripping, retry merge, falsy value preservation,
  no mutation of defaults
- 32 tests passing, lint clean, dist rebuilt
This commit is contained in:
Salman Chishti 2026-04-08 21:39:12 +00:00 committed by GitHub
parent 95933befc0
commit f9d72d3f45
8 changed files with 169 additions and 63 deletions

View file

@ -8,26 +8,28 @@ describe('callAsyncFunction', () => {
expect(result).toEqual('bar') expect(result).toEqual('bar')
}) })
test('passes getOctokit through the script context', async () => { test('passes createOctokit through the script context', async () => {
const getOctokit = jest.fn().mockReturnValue('secondary-client') const createOctokit = jest.fn().mockReturnValue('secondary-client')
const result = await callAsyncFunction( const result = await callAsyncFunction(
{getOctokit} as any, {createOctokit} as any,
"return getOctokit('token')" "return createOctokit('token')"
) )
expect(getOctokit).toHaveBeenCalledWith('token') expect(createOctokit).toHaveBeenCalledWith('token')
expect(result).toEqual('secondary-client') expect(result).toEqual('secondary-client')
}) })
test('getOctokit creates client independent from github', async () => { test('createOctokit creates client independent from github', async () => {
const github = {rest: {issues: 'primary'}} const github = {rest: {issues: 'primary'}}
const getOctokit = jest.fn().mockReturnValue({rest: {issues: 'secondary'}}) const createOctokit = jest
.fn()
.mockReturnValue({rest: {issues: 'secondary'}})
const result = await callAsyncFunction( const result = await callAsyncFunction(
{github, getOctokit} as any, {github, createOctokit} as any,
` `
const secondary = getOctokit('other-token') const secondary = createOctokit('other-token')
return { return {
primary: github.rest.issues, primary: github.rest.issues,
secondary: secondary.rest.issues, secondary: secondary.rest.issues,
@ -41,32 +43,32 @@ describe('callAsyncFunction', () => {
secondary: 'secondary', secondary: 'secondary',
different: true different: true
}) })
expect(getOctokit).toHaveBeenCalledWith('other-token') expect(createOctokit).toHaveBeenCalledWith('other-token')
}) })
test('getOctokit passes options through', async () => { test('createOctokit passes options through', async () => {
const getOctokit = jest.fn().mockReturnValue('client-with-opts') const createOctokit = jest.fn().mockReturnValue('client-with-opts')
const result = await callAsyncFunction( const result = await callAsyncFunction(
{getOctokit} as any, {createOctokit} as any,
`return getOctokit('my-token', { baseUrl: 'https://ghes.example.com/api/v3' })` `return createOctokit('my-token', { baseUrl: 'https://ghes.example.com/api/v3' })`
) )
expect(getOctokit).toHaveBeenCalledWith('my-token', { expect(createOctokit).toHaveBeenCalledWith('my-token', {
baseUrl: 'https://ghes.example.com/api/v3' baseUrl: 'https://ghes.example.com/api/v3'
}) })
expect(result).toEqual('client-with-opts') expect(result).toEqual('client-with-opts')
}) })
test('getOctokit supports plugins', async () => { test('createOctokit supports plugins', async () => {
const getOctokit = jest.fn().mockReturnValue('client-with-plugins') const createOctokit = jest.fn().mockReturnValue('client-with-plugins')
const result = await callAsyncFunction( const result = await callAsyncFunction(
{getOctokit} as any, {createOctokit} as any,
`return getOctokit('my-token', { previews: ['v3'] }, 'pluginA', 'pluginB')` `return createOctokit('my-token', { previews: ['v3'] }, 'pluginA', 'pluginB')`
) )
expect(getOctokit).toHaveBeenCalledWith( expect(createOctokit).toHaveBeenCalledWith(
'my-token', 'my-token',
{previews: ['v3']}, {previews: ['v3']},
'pluginA', 'pluginA',
@ -75,24 +77,24 @@ describe('callAsyncFunction', () => {
expect(result).toEqual('client-with-plugins') expect(result).toEqual('client-with-plugins')
}) })
test('multiple getOctokit calls produce independent clients', async () => { test('multiple createOctokit calls produce independent clients', async () => {
const getOctokit = jest const createOctokit = jest
.fn() .fn()
.mockReturnValueOnce({id: 'client-a'}) .mockReturnValueOnce({id: 'client-a'})
.mockReturnValueOnce({id: 'client-b'}) .mockReturnValueOnce({id: 'client-b'})
const result = await callAsyncFunction( const result = await callAsyncFunction(
{getOctokit} as any, {createOctokit} as any,
` `
const a = getOctokit('token-a') const a = createOctokit('token-a')
const b = getOctokit('token-b') const b = createOctokit('token-b')
return { a: a.id, b: b.id, different: a !== b } return { a: a.id, b: b.id, different: a !== b }
` `
) )
expect(getOctokit).toHaveBeenCalledTimes(2) expect(createOctokit).toHaveBeenCalledTimes(2)
expect(getOctokit).toHaveBeenNthCalledWith(1, 'token-a') expect(createOctokit).toHaveBeenNthCalledWith(1, 'token-a')
expect(getOctokit).toHaveBeenNthCalledWith(2, 'token-b') expect(createOctokit).toHaveBeenNthCalledWith(2, 'token-b')
expect(result).toEqual({a: 'client-a', b: 'client-b', different: true}) expect(result).toEqual({a: 'client-a', b: 'client-b', different: true})
}) })

View file

@ -76,6 +76,23 @@ describe('createConfiguredGetOctokit', () => {
) )
}) })
test('deep-merges retry so partial overrides preserve existing settings', () => {
const raw = makeMockGetOctokit()
const defaults = {
retry: {enabled: true, retries: 3}
}
const wrapped = createConfiguredGetOctokit(raw as any, defaults)
wrapped('tok' as any, {retry: {retries: 5}} as any)
expect(raw).toHaveBeenCalledWith(
'tok',
expect.objectContaining({
retry: {enabled: true, retries: 5}
})
)
})
test('user can override request.retries explicitly', () => { test('user can override request.retries explicitly', () => {
const raw = makeMockGetOctokit() const raw = makeMockGetOctokit()
const defaults = {request: {retries: 3}} const defaults = {request: {retries: 3}}
@ -158,15 +175,35 @@ describe('createConfiguredGetOctokit', () => {
test('baseUrl: undefined from user does not clobber default', () => { test('baseUrl: undefined from user does not clobber default', () => {
const raw = makeMockGetOctokit() const raw = makeMockGetOctokit()
const defaults = {baseUrl: 'https://api.github.com'} const defaults = {baseUrl: 'https://ghes.example.com/api/v3'}
const wrapped = createConfiguredGetOctokit(raw as any, defaults) const wrapped = createConfiguredGetOctokit(raw as any, defaults)
wrapped('tok' as any, {baseUrl: undefined} as any) wrapped('tok' as any, {baseUrl: undefined} as any)
// undefined spread still overwrites — this documents current behavior.
// The `baseUrl` key is present but value is undefined.
const calledOpts = raw.mock.calls[0][1] const calledOpts = raw.mock.calls[0][1]
expect(calledOpts).toHaveProperty('baseUrl') expect(calledOpts.baseUrl).toBe('https://ghes.example.com/api/v3')
})
test('undefined values in nested request are stripped', () => {
const raw = makeMockGetOctokit()
const defaults = {request: {retries: 3, agent: 'proxy'}}
const wrapped = createConfiguredGetOctokit(raw as any, defaults)
wrapped('tok' as any, {request: {retries: undefined, timeout: 5000}} as any)
const calledOpts = raw.mock.calls[0][1]
expect(calledOpts.request).toEqual({retries: 3, agent: 'proxy', timeout: 5000})
})
test('undefined values in nested retry are stripped', () => {
const raw = makeMockGetOctokit()
const defaults = {retry: {enabled: true, retries: 3}}
const wrapped = createConfiguredGetOctokit(raw as any, defaults)
wrapped('tok' as any, {retry: {enabled: undefined, retries: 5}} as any)
const calledOpts = raw.mock.calls[0][1]
expect(calledOpts.retry).toEqual({enabled: true, retries: 5})
}) })
test('each call creates an independent client', () => { test('each call creates an independent client', () => {
@ -183,4 +220,39 @@ describe('createConfiguredGetOctokit', () => {
expect(b).toBe('client-b') expect(b).toBe('client-b')
expect(raw).toHaveBeenCalledTimes(2) expect(raw).toHaveBeenCalledTimes(2)
}) })
test('does not mutate defaultOptions between calls', () => {
const raw = makeMockGetOctokit()
const defaults = {
request: {retries: 3},
retry: {enabled: true}
}
const originalDefaults = JSON.parse(JSON.stringify(defaults))
const wrapped = createConfiguredGetOctokit(raw as any, defaults)
wrapped('tok' as any, {request: {timeout: 5000}, retry: {retries: 10}} as any)
wrapped('tok' as any, {request: {timeout: 9000}} as any)
expect(defaults).toEqual(originalDefaults)
})
test('falsy-but-valid values are preserved, only undefined is stripped', () => {
const raw = makeMockGetOctokit()
const defaults = {baseUrl: 'https://ghes.example.com/api/v3'}
const wrapped = createConfiguredGetOctokit(raw as any, defaults)
wrapped('tok' as any, {
log: null,
retries: 0,
debug: false,
userAgent: ''
} as any)
const calledOpts = raw.mock.calls[0][1]
expect(calledOpts.log).toBeNull()
expect(calledOpts.retries).toBe(0)
expect(calledOpts.debug).toBe(false)
expect(calledOpts.userAgent).toBe('')
expect(calledOpts.baseUrl).toBe('https://ghes.example.com/api/v3')
})
}) })

View file

@ -3,12 +3,12 @@
import {getOctokit} from '@actions/github' import {getOctokit} from '@actions/github'
import {callAsyncFunction} from '../src/async-function' import {callAsyncFunction} from '../src/async-function'
describe('getOctokit integration via callAsyncFunction', () => { describe('createOctokit integration via callAsyncFunction', () => {
test('real getOctokit creates a functional Octokit client in script scope', async () => { test('real getOctokit creates a functional Octokit client in script scope', async () => {
const result = await callAsyncFunction( const result = await callAsyncFunction(
{getOctokit} as any, {createOctokit: getOctokit} as any,
` `
const client = getOctokit('fake-token-for-test') const client = createOctokit('fake-token-for-test')
return { return {
hasRest: typeof client.rest === 'object', hasRest: typeof client.rest === 'object',
hasGraphql: typeof client.graphql === 'function', hasGraphql: typeof client.graphql === 'function',
@ -32,9 +32,9 @@ describe('getOctokit integration via callAsyncFunction', () => {
const primary = getOctokit('primary-token') const primary = getOctokit('primary-token')
const result = await callAsyncFunction( const result = await callAsyncFunction(
{github: primary, getOctokit} as any, {github: primary, createOctokit: getOctokit} as any,
` `
const secondary = getOctokit('secondary-token') const secondary = createOctokit('secondary-token')
return { return {
bothHaveRest: typeof github.rest === 'object' && typeof secondary.rest === 'object', bothHaveRest: typeof github.rest === 'object' && typeof secondary.rest === 'object',
areDistinct: github !== secondary areDistinct: github !== secondary
@ -48,11 +48,11 @@ describe('getOctokit integration via callAsyncFunction', () => {
}) })
}) })
test('getOctokit accepts options for GHES base URL', async () => { test('createOctokit accepts options for GHES base URL', async () => {
const result = await callAsyncFunction( const result = await callAsyncFunction(
{getOctokit} as any, {createOctokit: getOctokit} as any,
` `
const client = getOctokit('fake-token', { const client = createOctokit('fake-token', {
baseUrl: 'https://ghes.example.com/api/v3' baseUrl: 'https://ghes.example.com/api/v3'
}) })
return typeof client.rest === 'object' return typeof client.rest === 'object'
@ -62,12 +62,12 @@ describe('getOctokit integration via callAsyncFunction', () => {
expect(result).toBe(true) expect(result).toBe(true)
}) })
test('multiple getOctokit calls produce independent clients with different tokens', async () => { test('multiple createOctokit calls produce independent clients with different tokens', async () => {
const result = await callAsyncFunction( const result = await callAsyncFunction(
{getOctokit} as any, {createOctokit: getOctokit} as any,
` `
const clientA = getOctokit('token-a') const clientA = createOctokit('token-a')
const clientB = getOctokit('token-b') const clientB = createOctokit('token-b')
return { return {
aHasRest: typeof clientA.rest === 'object', aHasRest: typeof clientA.rest === 'object',
bHasRest: typeof clientB.rest === 'object', bHasRest: typeof clientB.rest === 'object',

29
dist/index.js vendored
View file

@ -36189,13 +36189,22 @@ function callAsyncFunction(args, source) {
} }
;// CONCATENATED MODULE: ./src/create-configured-getoctokit.ts ;// CONCATENATED MODULE: ./src/create-configured-getoctokit.ts
/**
* Strip keys whose value is `undefined` so they don't clobber defaults
* during object spread (e.g. `{baseUrl: undefined}` would wipe a GHES URL).
*/
function stripUndefined(obj) {
return Object.fromEntries(Object.entries(obj).filter(([, v]) => v !== undefined));
}
/** /**
* Creates a wrapped getOctokit that inherits default options and plugins. * Creates a wrapped getOctokit that inherits default options and plugins.
* Secondary clients created via the wrapper get the same retry, logging, * Secondary clients created via the wrapper get the same retry, logging,
* orchestration ID, and retries count as the pre-built `github` client. * orchestration ID, and retries count as the pre-built `github` client.
* *
* - `request` is deep-merged so partial overrides (e.g. `{request: {timeout: 5000}}`) * - `request` and `retry` are deep-merged so partial overrides
* don't clobber inherited `retries`, proxy agent, or fetch defaults. * (e.g. `{request: {timeout: 5000}}`) don't clobber inherited values.
* - `undefined` values in user options are stripped to prevent accidental
* clobbering of defaults (e.g. GHES `baseUrl`).
* - Default plugins (retry, requestLog) are always included; duplicates are skipped. * - Default plugins (retry, requestLog) are always included; duplicates are skipped.
*/ */
function createConfiguredGetOctokit(rawGetOctokit, defaultOptions, function createConfiguredGetOctokit(rawGetOctokit, defaultOptions,
@ -36203,15 +36212,17 @@ function createConfiguredGetOctokit(rawGetOctokit, defaultOptions,
...defaultPlugins) { ...defaultPlugins) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
return ((token, options, ...plugins) => { return ((token, options, ...plugins) => {
var _a, _b; var _a, _b, _c, _d;
const userOpts = options || {}; const userOpts = stripUndefined(options !== null && options !== void 0 ? options : {});
const defaultRequest = (_a = defaultOptions.request) !== null && _a !== void 0 ? _a : {}; const defaultRequest = (_a = defaultOptions.request) !== null && _a !== void 0 ? _a : {};
const userRequest = (_b = userOpts.request) !== null && _b !== void 0 ? _b : {}; const userRequest = stripUndefined((_b = userOpts.request) !== null && _b !== void 0 ? _b : {});
const defaultRetry = (_c = defaultOptions.retry) !== null && _c !== void 0 ? _c : {};
const userRetry = stripUndefined((_d = userOpts.retry) !== null && _d !== void 0 ? _d : {});
const merged = { const merged = {
...defaultOptions, ...defaultOptions,
...userOpts, ...userOpts,
// Deep-merge `request` to preserve retries, proxy agent, and fetch request: { ...defaultRequest, ...userRequest },
request: { ...defaultRequest, ...userRequest } retry: { ...defaultRetry, ...userRetry }
}; };
// Deduplicate: default plugins first, then user plugins that aren't already present // Deduplicate: default plugins first, then user plugins that aren't already present
const allPlugins = [...defaultPlugins]; const allPlugins = [...defaultPlugins];
@ -36322,14 +36333,14 @@ async function main() {
const script = core.getInput('script', { required: true }); const script = core.getInput('script', { required: true });
// Wrap getOctokit so secondary clients inherit retry, logging, // Wrap getOctokit so secondary clients inherit retry, logging,
// orchestration ID, and the action's retries input. // orchestration ID, and the action's retries input.
const configuredGetOctokit = createConfiguredGetOctokit(lib_github.getOctokit, opts, plugin_retry_dist_node.retry, dist_node.requestLog); const configuredGetOctokit = createConfiguredGetOctokit(lib_github.getOctokit, { ...opts }, plugin_retry_dist_node.retry, dist_node.requestLog);
// Using property/value shorthand on `require` (e.g. `{require}`) causes compilation errors. // Using property/value shorthand on `require` (e.g. `{require}`) causes compilation errors.
const result = await callAsyncFunction({ const result = await callAsyncFunction({
require: wrapRequire, require: wrapRequire,
__original_require__: require, __original_require__: require,
github, github,
octokit: github, octokit: github,
getOctokit: configuredGetOctokit, createOctokit: configuredGetOctokit,
context: lib_github.context, context: lib_github.context,
core: core, core: core,
exec: exec, exec: exec,

View file

@ -13,7 +13,7 @@ export declare type AsyncFunctionArguments = {
core: typeof core core: typeof core
github: InstanceType<typeof GitHub> github: InstanceType<typeof GitHub>
octokit: InstanceType<typeof GitHub> octokit: InstanceType<typeof GitHub>
getOctokit: typeof getOctokit createOctokit: typeof getOctokit
exec: typeof exec exec: typeof exec
glob: typeof glob glob: typeof glob
io: typeof io io: typeof io

View file

@ -1,12 +1,26 @@
import {getOctokit} from '@actions/github' import {getOctokit} from '@actions/github'
/**
* Strip keys whose value is `undefined` so they don't clobber defaults
* during object spread (e.g. `{baseUrl: undefined}` would wipe a GHES URL).
*/
function stripUndefined(
obj: Record<string, unknown>
): Record<string, unknown> {
return Object.fromEntries(
Object.entries(obj).filter(([, v]) => v !== undefined)
)
}
/** /**
* Creates a wrapped getOctokit that inherits default options and plugins. * Creates a wrapped getOctokit that inherits default options and plugins.
* Secondary clients created via the wrapper get the same retry, logging, * Secondary clients created via the wrapper get the same retry, logging,
* orchestration ID, and retries count as the pre-built `github` client. * orchestration ID, and retries count as the pre-built `github` client.
* *
* - `request` is deep-merged so partial overrides (e.g. `{request: {timeout: 5000}}`) * - `request` and `retry` are deep-merged so partial overrides
* don't clobber inherited `retries`, proxy agent, or fetch defaults. * (e.g. `{request: {timeout: 5000}}`) don't clobber inherited values.
* - `undefined` values in user options are stripped to prevent accidental
* clobbering of defaults (e.g. GHES `baseUrl`).
* - Default plugins (retry, requestLog) are always included; duplicates are skipped. * - Default plugins (retry, requestLog) are always included; duplicates are skipped.
*/ */
export function createConfiguredGetOctokit( export function createConfiguredGetOctokit(
@ -17,18 +31,25 @@ export function createConfiguredGetOctokit(
): typeof getOctokit { ): typeof getOctokit {
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
return ((token: string, options?: any, ...plugins: any[]) => { return ((token: string, options?: any, ...plugins: any[]) => {
const userOpts = options || {} const userOpts = stripUndefined(options ?? {})
const defaultRequest = const defaultRequest =
(defaultOptions.request as Record<string, unknown> | undefined) ?? {} (defaultOptions.request as Record<string, unknown> | undefined) ?? {}
const userRequest = const userRequest = stripUndefined(
(userOpts.request as Record<string, unknown> | undefined) ?? {} (userOpts.request as Record<string, unknown> | undefined) ?? {}
)
const defaultRetry =
(defaultOptions.retry as Record<string, unknown> | undefined) ?? {}
const userRetry = stripUndefined(
(userOpts.retry as Record<string, unknown> | undefined) ?? {}
)
const merged = { const merged = {
...defaultOptions, ...defaultOptions,
...userOpts, ...userOpts,
// Deep-merge `request` to preserve retries, proxy agent, and fetch request: {...defaultRequest, ...userRequest},
request: {...defaultRequest, ...userRequest} retry: {...defaultRetry, ...userRetry}
} }
// Deduplicate: default plugins first, then user plugins that aren't already present // Deduplicate: default plugins first, then user plugins that aren't already present

View file

@ -64,7 +64,7 @@ async function main(): Promise<void> {
// orchestration ID, and the action's retries input. // orchestration ID, and the action's retries input.
const configuredGetOctokit = createConfiguredGetOctokit( const configuredGetOctokit = createConfiguredGetOctokit(
getOctokit, getOctokit,
opts, {...opts},
retry, retry,
requestLog requestLog
) )
@ -76,7 +76,7 @@ async function main(): Promise<void> {
__original_require__: __non_webpack_require__, __original_require__: __non_webpack_require__,
github, github,
octokit: github, octokit: github,
getOctokit: configuredGetOctokit, createOctokit: configuredGetOctokit,
context, context,
core, core,
exec, exec,

View file

@ -11,7 +11,7 @@ export declare type AsyncFunctionArguments = {
core: typeof core; core: typeof core;
github: InstanceType<typeof GitHub>; github: InstanceType<typeof GitHub>;
octokit: InstanceType<typeof GitHub>; octokit: InstanceType<typeof GitHub>;
getOctokit: typeof getOctokit; createOctokit: typeof getOctokit;
exec: typeof exec; exec: typeof exec;
glob: typeof glob; glob: typeof glob;
io: typeof io; io: typeof io;