Skip to content

feat(webex-core): option to skip catalog request when unauthenticated - #5170

Open
Coread wants to merge 7 commits into
nextfrom
skip_catalog_fetch_when_unauthenticated
Open

feat(webex-core): option to skip catalog request when unauthenticated#5170
Coread wants to merge 7 commits into
nextfrom
skip_catalog_fetch_when_unauthenticated

Conversation

@Coread

@Coread Coread commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

COMPLETES #< INSERT LINK TO ISSUE >

This pull request addresses

Add an option to skip the by-proximity catalog request if the user is unauthenticated

by making the following changes

< DESCRIBE YOUR CHANGES >

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios were tested

< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >

The GAI Coding Policy And Copyright Annotation Best Practices

  • GAI was not used (or, no additional notation is required)
  • Code was generated entirely by GAI
  • GAI was used to create a draft that was subsequently customized or modified
  • Coder created a draft manually that was non-substantively modified by GAI (e.g., refactoring was performed by GAI on manually written code)
  • Tool used for AI assistance (GitHub Copilot / Other - specify)
    • Github Copilot
    • Other - Please Specify
  • This PR is related to
    • Feature
    • Defect fix
    • Tech Debt
    • Automation

I certified that

  • I have read and followed contributing guidelines
  • I discussed changes with code owners prior to submitting this pull request
  • I have not skipped any automated checks
  • All existing and new tests passed
  • I have updated the documentation accordingly

Make sure to have followed the contributing guidelines before submitting.

@Coread
Coread requested review from a team as code owners August 13, 2026 13:26

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c913e0a30

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

services.initialize();
services.listenToOnce.getCall(0).args[2]();
services.listenToOnce.getCall(1).args[2]();
await waitForAsync();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Advance the fake clock before waiting on setImmediate

With sinon.useFakeTimers() installed, the waitForAsync() helper's setImmediate is faked, so this await cannot resolve until the fake clock is advanced; however clock.tickAsync(30_000) is only reached after the await. This makes the new skip-preauth unit test hang in CI (the mirrored services-v2 test has the same pattern). Move the async flush before installing fake timers, or advance the fake clock before awaiting it.

Useful? React with 👍 / 👎.

@Coread Coread added the validated If the pull request is validated for automation. label Aug 17, 2026

@jor-row jor-row left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please fill out pr description and fix hanging test

tested locally and works well otherwise

services.initialize();
services.listenToOnce.getCall(0).args[2]();
services.listenToOnce.getCall(1).args[2]();
await waitForAsync();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this line is not needed

services.initialize();
services.listenToOnce.getCall(0).args[2]();
services.listenToOnce.getCall(1).args[2]();
await waitForAsync();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this line is not needed

@github-actions github-actions Bot removed the validated If the pull request is validated for automation. label Aug 17, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4504c98d2a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

this.logger.info(
'services: skipping preauth catalog collection while unauthenticated as per the config'
);
this._finalizeReady();

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 Badge Register the auth listener before finalizing readiness

When gated initialization and skipPreauthCatalogOnUnauthenticated are enabled without a credentials refresh in progress, _finalizeReady() sets services.ready synchronously, which can synchronously emit webex.ready. If a consumer's ready handler installs a token immediately, change:canAuthorize fires before the listener below is registered, so initServiceCatalogs() never runs and the authenticated catalog remains unavailable. Register the fresh-login listener before calling _finalizeReady() in this branch.

Useful? React with 👍 / 👎.

@Coread Coread added the validated If the pull request is validated for automation. label Aug 19, 2026
@github-actions github-actions Bot removed the validated If the pull request is validated for automation. label Aug 19, 2026
@Coread Coread added the validated If the pull request is validated for automation. label Aug 19, 2026
@github-actions github-actions Bot removed the validated If the pull request is validated for automation. label Aug 21, 2026
@Coread Coread added the validated If the pull request is validated for automation. label Aug 21, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6aec33b02b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

}
stopServer();
// eslint-disable-next-line global-require, @typescript-eslint/no-var-requires
require('why-is-node-running')();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Invoke the package's default ESM export

why-is-node-running@3.2.2 is an ES module whose diagnostic function is the default export, so under the repository-required Node 22.14 runtime require('why-is-node-running') returns a module namespace object rather than a callable function. Every successful legacy Mocha run reaches this callback and then throws TypeError: require(...) is not a function, turning passing test runs into failures; import the default export or remove this temporary debug hook.

AGENTS.md reference: AGENTS.md:L1-L5

Useful? React with 👍 / 👎.

@Coread
Coread force-pushed the skip_catalog_fetch_when_unauthenticated branch from 6aec33b to 2928717 Compare August 21, 2026 12:38
@github-actions github-actions Bot removed the validated If the pull request is validated for automation. label Aug 21, 2026
@Coread Coread added the validated If the pull request is validated for automation. label Aug 21, 2026
@github-actions github-actions Bot removed the validated If the pull request is validated for automation. label Aug 21, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 369e94105f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


/* eslint-disable no-underscore-dangle */
describe('webex-core', () => {
describe.skip('webex-core', () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Re-enable the service integration suites

Changing the outer suite to describe.skip prevents every Services integration test in this file from running, so test:integration can pass without exercising catalog discovery, caching, initialization, or the new readiness behavior. The mirrored ServicesV2 suite is also disabled at test/integration/spec/services-v2/services-v2.js:31, and both files separately skip the gated-readiness test; restore the outer describe and the inner it in both suites.

Useful? React with 👍 / 👎.

@Coread Coread added the validated If the pull request is validated for automation. label Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

validated If the pull request is validated for automation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants