Skip to content

Conversation

@jbedard
Copy link
Member

@jbedard jbedard commented Nov 1, 2025


Changes are visible to end-users: yes/no

  • Searched for relevant documentation and updated as needed: yes/no
  • Breaking change (forces users to change their own code or config): yes/no
  • Suggested release notes appear below: yes/no

Test plan

  • Covered by existing test cases
  • New test cases added
  • Manual testing; please provide instructions so we can reproduce:

@aspect-workflows
Copy link

aspect-workflows bot commented Nov 1, 2025

Bazel 7 (Test)

⚠️ Buildkite build #11594 failed.

Failed tests (2)
//js/private/watch:watch_checked_0_test [k8-fastbuild] 🔗
//js/private/watch:watch_checked_1_test [k8-fastbuild] 🔗

💡 To reproduce the test failures, run

bazel test //js/private/watch:watch_checked_0_test //js/private/watch:watch_checked_1_test

Bazel 8 (Test)

Buildkite build #11594 is running...


Bazel 7 (Test)

e2e/bzlmod

All tests were cache hits

5 tests (100.0%) were fully cached saving 610ms.


Bazel 7 (Test)

e2e/git_dep_metadata

All tests were cache hits

1 test (100.0%) was fully cached saving 23ms.


Bazel 7 (Test)

e2e/git_dep_no_tar

All tests were cache hits

1 test (100.0%) was fully cached saving 24ms.


Bazel 7 (Test)

e2e/gyp_no_install_script

All tests were cache hits

2 tests (100.0%) were fully cached saving 151ms.


Bazel 7 (Test)

e2e/js_image_oci

⚠️ Buildkite build #11594 failed.


Bazel 7 (Test)

e2e/npm_link_package

All tests were cache hits

2 tests (100.0%) were fully cached saving 203ms.


Bazel 7 (Test)

e2e/npm_link_package-esm

All tests were cache hits

2 tests (100.0%) were fully cached saving 208ms.


Bazel 7 (Test)

e2e/npm_link_package-rerooted

All tests were cache hits

2 tests (100.0%) were fully cached saving 189ms.


Bazel 7 (Test)

e2e/npm_translate_lock

All tests were cache hits

3 tests (100.0%) were fully cached saving 1s.


Bazel 7 (Test)

e2e/npm_translate_lock_disable_hooks

All tests were cache hits

3 tests (100.0%) were fully cached saving 259ms.


Bazel 7 (Test)

e2e/npm_translate_lock_empty

All tests were cache hits

2 tests (100.0%) were fully cached saving 122ms.


Bazel 7 (Test)

e2e/npm_translate_lock_exclude_package_contents

All tests were cache hits

1 test (100.0%) was fully cached saving 34ms.


Bazel 7 (Test)

e2e/npm_translate_lock_link_workspace

All tests were cache hits

2 tests (100.0%) were fully cached saving 219ms.


Bazel 7 (Test)

e2e/npm_translate_lock_multi

All tests were cache hits

2 tests (100.0%) were fully cached saving 164ms.


Bazel 7 (Test)

e2e/npm_translate_lock_partial_clone

All tests were cache hits

1 test (100.0%) was fully cached saving 61ms.


Bazel 7 (Test)

e2e/npm_translate_lock_replace_packages

All tests were cache hits

4 tests (100.0%) were fully cached saving 483ms.


Bazel 7 (Test)

e2e/npm_translate_lock_subdir_patch

All tests were cache hits

1 test (100.0%) was fully cached saving 77ms.


Bazel 7 (Test)

e2e/npm_translate_package_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.


Bazel 7 (Test)

e2e/npm_translate_yarn_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.


Bazel 7 (Test)

e2e/package_json_module

All tests were cache hits

1 test (100.0%) was fully cached saving 316ms.


Bazel 7 (Test)

e2e/patch_from_repo

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.


Bazel 7 (Test)

e2e/pnpm_lockfiles

All tests were cache hits

82 tests (100.0%) were fully cached saving 9s.


Bazel 7 (Test)

e2e/pnpm_repo_install

All tests were cache hits

1 test (100.0%) was fully cached saving 1s.


Bazel 7 (Test)

e2e/pnpm_version

All tests were cache hits

1 test (100.0%) was fully cached saving 97ms.


Bazel 7 (Test)

e2e/pnpm_workspace

All tests were cache hits

15 tests (100.0%) were fully cached saving 3s.


Bazel 7 (Test)

e2e/pnpm_workspace_deps

All tests were cache hits

3 tests (100.0%) were fully cached saving 425ms.


Bazel 7 (Test)

e2e/pnpm_workspace_rerooted

All tests were cache hits

15 tests (100.0%) were fully cached saving 3s.


Bazel 7 (Test)

e2e/repo_mapping

All tests were cache hits

3 tests (100.0%) were fully cached saving 395ms.


Bazel 7 (Test)

e2e/runfiles

All tests were cache hits

1 test (100.0%) was fully cached saving 105ms.


Bazel 7 (Test)

e2e/stamped_package_json

All tests were cache hits

1 test (100.0%) was fully cached saving 44ms.


Bazel 7 (Test)

e2e/vendored_node

All tests were cache hits

1 test (100.0%) was fully cached saving 103ms.


Bazel 7 (Test)

e2e/vendored_tarfile

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.


Bazel 7 (Test)

e2e/verify_patches

All tests were cache hits

2 tests (100.0%) were fully cached saving 109ms.


Bazel 7 (Test)

e2e/worker

All tests were cache hits

1 test (100.0%) was fully cached saving 35ms.


Bazel 7 (Test)

e2e/workspace

All tests were cache hits

1 test (100.0%) was fully cached saving 35ms.


Buildifier      Format

*/
async awaitFirstCycle() {
if (this._version > 0) {
await this._send<SubscribeMessage>(MessageType.SUBSCRIBE, {
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe this is just re-inventing the CAP message from the original proposal?

@thesayyn were you thinking that a socket has a single subscription, and the CAP message is essentially declaring the options for that socket/subscription? This PR does similar but "subscribe" seems to imply that you can have more then one subscription per socket - which is probably more complicated and not worth it?

Copy link
Member

Choose a reason for hiding this comment

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

my worry with SUBSCRIBE is that it blocks cycle messages from being sent until the implementor tells us what it needs. the name implies that it can be sent N many times and every subscription needs an id to distinguish the CYCLE events from each other.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm starting to think we should keep the existing behaviour where 1 socket = 1 subscription, and this PR (and the associated protocol spec changes) should instead implement the original CAP message - which I think aligns with your original idea?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, and revisit the scope idea where as part of the CAP, we can decide what the user is asking for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants