Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 673d4f7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 43 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
d-gubert
force-pushed
the
feat/apps-engine-split--pr2d-remove-apps-engine-files
branch
from
a81d286 to
a908c44
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40343 +/- ##
===========================================
- Coverage 69.67% 69.60% -0.07%
===========================================
Files 3317 3317
Lines 121867 121868 +1
Branches 21781 21761 -20
===========================================
- Hits 84907 84831 -76
- Misses 33652 33713 +61
- Partials 3308 3324 +16
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
d-gubert
force-pushed
the
feat/apps-engine-split--pr2c-flip-the-switch
branch
from
6bc4e83 to
b8cb7a9
Compare
d-gubert
force-pushed
the
feat/apps-engine-split--pr2d-remove-apps-engine-files
branch
from
aff63fd to
a161706
Compare
This was referenced May 4, 2026
d-gubert
force-pushed
the
feat/apps-engine-split--pr2c-flip-the-switch
branch
from
b8cb7a9 to
a7dcbfc
Compare
d-gubert
force-pushed
the
feat/apps-engine-split--pr2d-remove-apps-engine-files
branch
from
a161706 to
c16e991
Compare
d-gubert
changed the title
ggazzo
changed the base branch from
feat/apps-engine-split--pr2c-flip-the-switch
to
develop
ggazzo
reviewed
May 7, 2026
Adds `src/definition/version.ts` which reads the package version via
`resolveJsonModule` and exports it as `ENGINE_VERSION`.
Replaces `AppPackageParser.getEngineVersion()` — which resolved the
version by traversing the filesystem relative to `__dirname` — with a
direct import of `ENGINE_VERSION`. This removes the assumption that
`package.json` lives at a predictable relative path, which will break
when `AppPackageParser` moves to a different package during the
apps-engine split.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
fix(apps-engine): fix ENGINE_VERSION runtime path in compiled output
Static `import { version } from '../../package.json'` resolves correctly
during TypeScript compilation (source lives at src/definition/) but the
emitted require('../../package.json') exits the package root at runtime
once compiled to definition/version.js (outDir='.', rootDir='./src').
Switching to require('../package.json') — which is the correct path
relative to the compiled output — and bypassing TypeScript's compile-time
module resolution avoids the path mismatch entirely.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copies src/server/ from @rocket.chat/apps-engine verbatim, then rewrites all relative definition/ imports to package imports (`@rocket.chat/apps-engine/definition/...`). apps-engine still contains its server code at this point — this is an additive copy only. The deletion happens in a later PR once @packages/apps is confirmed working independently. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copies src/client/ (AppClientManager, AppsEngineUIHost, AppsEngineUIClient) from @rocket.chat/apps-engine, rewriting relative definition/ imports to package imports. This code is a known rough edge: browser-side UI host logic does not semantically belong in a server orchestration package. It is consolidated here for pragmatic simplicity during the apps-engine split. A future @rocket.chat/apps-client package is tracked in the TODO comment added to src/client/index.ts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copies deno-runtime/ verbatim from @rocket.chat/apps-engine. The import map in deno.jsonc still points to ./../src/ which is only valid in the current location (apps-engine). Making the import map location-independent (using a runtime-generated map) is handled in a dedicated follow-up PR to keep the diff focused. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… directly The old src/bridges/IListenerBridge.ts used module augmentation (`declare module '@rocket.chat/apps-engine/server/bridges'`) to extend IListenerBridge with core-typings-specific overloads. Now that IListenerBridge lives in this package, the augmentation workaround is no longer needed. The extra overload signatures are merged directly into src/server/bridges/IListenerBridge.ts and the augmentation file is deleted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Strip @rocket.chat/apps-engine down to the definition layer only. All runtime code (AppManager, bridges, compiler, deno-runtime, etc.) has already been moved to @rocket.chat/apps in previous commits. Updates package.json, tsconfig.json, and turbo.json to reflect the narrower scope (definition/** and lib/** outputs only). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These files were already moved to @rocket.chat/apps/src/client but were not deleted from apps-engine in the previous cleanup commit. They caused TypeScript build errors because their dependencies (constants, utils, definition sub-files) had been removed. Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com>
These files were already moved to @rocket.chat/apps/src/server but were not deleted from apps-engine in the previous cleanup commit. They caused TypeScript build errors because many of their dependencies (bridges, managers, errors, storage types, etc.) had been removed from apps-engine. Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com>
d-gubert
force-pushed
the
feat/apps-engine-split--pr2d-remove-apps-engine-files
branch
from
6d138b7 to
673d4f7
Compare
ggazzo
approved these changes
May 7, 2026
ggazzo
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
🔴 Layne — 3 finding(s)Found 3 issue(s): 3 high. |
Member
Author
|
/layne exception-approve LAYNE-c62cb997f4757617 LAYNE-b7a114d497e516af LAYNE-8fde114514b41f27 reason: executed command is always Deno, as it has always been |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
More details available here
Issue(s)
ARCH-2129
Steps to test or reproduce
Further comments
Related to the "Apps-Engine split" stack:
@rocket.chat/apps-engineto@rocket.chat/appsinternal package #40395Deprecated