Skip to content

Async import of the appStore packages #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Frawatson wants to merge 1 commit into
base: appstore-sync-refactor-base
Choose a base branch
from
Open

Async import of the appStore packages #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions packages/app-store/_utils/getCalendar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import appStore from "..";

const log = logger.getChildLogger({ prefix: ["CalendarManager"] });

export const getCalendar = (credential: CredentialPayload | null): Calendar | null => {
export const getCalendar = async (credential: CredentialPayload | null): Promise<Calendar | null> => {
if (!credential || !credential.key) return null;
let { type: calendarType } = credential;

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Error handling for dynamic imports (confidence: 93%)

If a dynamic import() in the appStore fails (e.g., module not found for a corrupted or removed app), the await on line 11 will throw an unhandled rejection. Since getCalendar() is called in many places including loops, a single failed import could crash the entire operation. Consider wrapping the await in a try/catch that returns null on failure, consistent with the existing null-return pattern.

Evidence:

  • Line 11: const calendarApp = await appStore[calendarType.split('_').join('') as keyof typeof appStore]
  • If the import() Promise rejects, the error propagates up to whichever caller invoked getCalendar()
  • getCachedResults() and getConnectedCalendars() iterate over multiple credentials — one bad import would abort the entire batch

Agent: architecture

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (12 lines, 1 file) — review recommended)

If a dynamic import() in the appStore fails (e.g., module not found for a corrupted or removed app), the await on line 11 will throw an unhandled rejection. Since getCalendar() is called in many places including loops, a single failed import could crash the entire operation. Consider wrapping the await in a try/catch that returns null on failure, consistent with the existing null-return pattern.

--- a/packages/app-store/_utils/getCalendar.ts
+++ b/packages/app-store/_utils/getCalendar.ts
@@ -9,7 +9,14 @@ export const getCalendar = async (credential: CredentialPayload | null): Promise
   if (!credential || !credential.key) return null;
   let { type: calendarType } = credential;
   if (calendarType?.endsWith("_other_calendar")) {
     calendarType = calendarType.split("_other_calendar")[0];
   }
-  const calendarApp = await appStore[calendarType.split("_").join("") as keyof typeof appStore];
+  // Wrap the dynamic import in try/catch so that a missing or corrupted app
+  // module does not abort callers that iterate over multiple credentials
+  // (e.g. getCachedResults, getConnectedCalendars). Consistent with the
+  // null-return pattern used below for unimplemented calendar types.
+  let calendarApp: Awaited<(typeof appStore)[keyof typeof appStore]> | undefined;
+  try {
+    calendarApp = await appStore[calendarType.split("_").join("") as keyof typeof appStore];
+  } catch (e) {
+    log.warn(`Failed to load calendar app for type ${calendarType}`, e);
+    return null;
+  }
   if (!(calendarApp && "lib" in calendarApp && "CalendarService" in calendarApp.lib)) {
     log.warn(`calendar of type ${calendarType} is not implemented`);
     return null;

🤖 Grapple PR auto-fix • minor • Review this diff before applying

if (calendarType?.endsWith("_other_calendar")) {

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Error handling (confidence: 93%)

If a dynamic import() fails (e.g., module not found at runtime), the await on line 12 will throw an unhandled rejection. Since getCalendar is called in many places and callers may not always have adequate error handling for import failures, this could cause unexpected crashes. Consider wrapping the await in a try/catch that returns null on failure, consistent with the existing pattern of returning null for unknown calendar types.

Evidence:

  • Line 12: const calendarApp = await appStore[calendarType.split('_').join('') as keyof typeof appStore];
  • If the key doesn't exist in appStore, this would be await undefined which is fine, but a failed import() would throw
  • Intent specification notes: 'If an appStore dynamic import() fails, the error may propagate as an unhandled rejection'

Agent: logic

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ [Grapple PR] Auto-fix — logic agent (Small fix (8 lines, 1 file))

If a dynamic import() fails (e.g., module not found at runtime), the await on line 12 will throw an unhandled rejection. Since getCalendar is called in many places and callers may not always have adequate error handling for import failures, this could cause unexpected crashes. Consider wrapping the await in a try/catch that returns null on failure, consistent with the existing pattern of returning null for unknown calendar types.

Suggested change
if (calendarType?.endsWith("_other_calendar")) {
let calendarApp;
try {
calendarApp = await appStore[calendarType.split("_").join("") as keyof typeof appStore];
} catch (e) {
log.error(`Failed to load calendar app for type ${calendarType}`, e);
return null;
}

🤖 Grapple PR auto-fix • minor • confidence: 93%

calendarType = calendarType.split("_other_calendar")[0];
}
const calendarApp = appStore[calendarType.split("_").join("") as keyof typeof appStore];
const calendarApp = await appStore[calendarType.split("_").join("") as keyof typeof appStore];
if (!(calendarApp && "lib" in calendarApp && "CalendarService" in calendarApp.lib)) {
log.warn(`calendar of type ${calendarType} is not implemented`);
return null;
Expand Down
88 changes: 29 additions & 59 deletions packages/app-store/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,63 +1,33 @@
// import * as example from "./_example";
import * as applecalendar from "./applecalendar";
import * as caldavcalendar from "./caldavcalendar";
import * as closecom from "./closecom";
import * as dailyvideo from "./dailyvideo";
import * as exchange2013calendar from "./exchange2013calendar";
import * as exchange2016calendar from "./exchange2016calendar";
import * as exchangecalendar from "./exchangecalendar";
import * as facetime from "./facetime";
import * as giphy from "./giphy";
import * as googlecalendar from "./googlecalendar";
import * as googlevideo from "./googlevideo";
import * as hubspot from "./hubspot";
import * as huddle01video from "./huddle01video";
import * as jitsivideo from "./jitsivideo";
import * as larkcalendar from "./larkcalendar";
import * as office365calendar from "./office365calendar";
import * as office365video from "./office365video";
import * as plausible from "./plausible";
import * as salesforce from "./salesforce";
import * as sendgrid from "./sendgrid";
import * as stripepayment from "./stripepayment";
import * as sylapsvideo from "./sylapsvideo";
import * as tandemvideo from "./tandemvideo";
import * as vital from "./vital";
import * as wipemycalother from "./wipemycalother";
import * as zapier from "./zapier";
import * as zohocrm from "./zohocrm";
import * as zoomvideo from "./zoomvideo";

const appStore = {
// example,
applecalendar,
caldavcalendar,
closecom,
dailyvideo,
googlecalendar,
googlevideo,
hubspot,
huddle01video,
jitsivideo,
sylapsvideo,
larkcalendar,
office365calendar,
office365video,
plausible,
salesforce,
zohocrm,
sendgrid,
stripepayment,
tandemvideo,
vital,
zoomvideo,
wipemycalother,
giphy,
zapier,
exchange2013calendar,
exchange2016calendar,
exchangecalendar,
facetime,
// example: import("./example"),
applecalendar: import("./applecalendar"),
caldavcalendar: import("./caldavcalendar"),
closecom: import("./closecom"),
dailyvideo: import("./dailyvideo"),
googlecalendar: import("./googlecalendar"),
googlevideo: import("./googlevideo"),
hubspot: import("./hubspot"),
huddle01video: import("./huddle01video"),
jitsivideo: import("./jitsivideo"),
larkcalendar: import("./larkcalendar"),
office365calendar: import("./office365calendar"),
office365video: import("./office365video"),
plausible: import("./plausible"),
salesforce: import("./salesforce"),
zohocrm: import("./zohocrm"),
sendgrid: import("./sendgrid"),
stripepayment: import("./stripepayment"),
tandemvideo: import("./tandemvideo"),
vital: import("./vital"),
zoomvideo: import("./zoomvideo"),
wipemycalother: import("./wipemycalother"),
giphy: import("./giphy"),
zapier: import("./zapier"),
exchange2013calendar: import("./exchange2013calendar"),
exchange2016calendar: import("./exchange2016calendar"),
exchangecalendar: import("./exchangecalendar"),
facetime: import("./facetime"),
sylapsvideo: import("./sylapsvideo"),
};

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — No lazy loading benefit (confidence: 100%)

The import() calls are executed immediately at module evaluation time (when the appStore object is constructed), not lazily on first access. Every dynamic import is invoked eagerly as part of the object literal initialization, meaning all app packages are loaded as soon as appStore/index.ts is first imported. This provides no code-splitting or lazy-loading benefit over static imports — it only converts the values from modules to Promises of modules, adding async overhead without the intended benefit.

Evidence:

  • All import() calls are in the object literal value positions, meaning they execute when the object is created
  • The module is evaluated once on first import, at which point all 28+ dynamic imports fire simultaneously
  • For actual lazy loading, the imports should be wrapped in functions: applecalendar: () => import('./applecalendar') and called at the point of use
  • Intent specification notes: 'appStore entries are top-level dynamic imports evaluated at module load time (not lazily)'

Agent: logic

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — No actual lazy loading benefit (confidence: 100%)

The import() calls are executed eagerly at module evaluation time (when index.ts is first imported), not lazily on demand. The appStore object literal evaluates all its property initializers immediately, so every dynamic import() fires at once during module load. This provides no code-splitting or lazy-loading benefit — all app modules are still loaded upfront, just asynchronously. The refactor adds significant complexity (async propagation through ~12 files) without achieving the stated goal of lazy loading.

Evidence:

  • All import() expressions are in the object literal initializer, which runs at module evaluation time
  • For actual lazy loading, the imports should be wrapped in functions: applecalendar: () => import('./applecalendar') and called only when needed
  • The Promises are created eagerly and cached in the module-level appStore constant

Agent: architecture

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Missing app store entry (confidence: 100%)

The sylapsvideo entry has been moved from its original alphabetical/grouped position to the very end of the object (line 30). While it IS present in the new code, the reordering combined with it being the last entry makes it easy to accidentally drop in future edits. More importantly, verify this was intentional — the original code had sylapsvideo between jitsivideo and larkcalendar, and the new code places it after facetime.

Evidence:

  • Original order had sylapsvideo at line 10 (between jitsivideo and larkcalendar)
  • New code has sylapsvideo: import('./sylapsvideo') as the last entry at line 30
  • The intent specification flagged this as a risk: 'sylapsvideo entry present in appStore index.ts (it appears in original but may be missing from diff)'

Agent: architecture

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (58 lines, 1 file) — review recommended)

The import() calls are executed immediately at module evaluation time (when the appStore object is constructed), not lazily on first access. Every dynamic import is invoked eagerly as part of the object literal initialization, meaning all app packages are loaded as soon as appStore/index.ts is first imported. This provides no code-splitting or lazy-loading benefit over static imports — it only converts the values from modules to Promises of modules, adding async overhead without the intended benefit.

--- a/packages/app-store/index.ts
+++ b/packages/app-store/index.ts
@@ -1,31 +1,31 @@
 const appStore = {
-  // example: import("./example"),
-  applecalendar: import("./applecalendar"),
-  caldavcalendar: import("./caldavcalendar"),
-  closecom: import("./closecom"),
-  dailyvideo: import("./dailyvideo"),
-  googlecalendar: import("./googlecalendar"),
-  googlevideo: import("./googlevideo"),
-  hubspot: import("./hubspot"),
-  huddle01video: import("./huddle01video"),
-  jitsivideo: import("./jitsivideo"),
-  larkcalendar: import("./larkcalendar"),
-  office365calendar: import("./office365calendar"),
-  office365video: import("./office365video"),
-  plausible: import("./plausible"),
-  salesforce: import("./salesforce"),
-  zohocrm: import("./zohocrm"),
-  sendgrid: import("./sendgrid"),
-  stripepayment: import("./stripepayment"),
-  tandemvideo: import("./tandemvideo"),
-  vital: import("./vital"),
-  zoomvideo: import("./zoomvideo"),
-  wipemycalother: import("./wipemycalother"),
-  giphy: import("./giphy"),
-  zapier: import("./zapier"),
-  exchange2013calendar: import("./exchange2013calendar"),
-  exchange2016calendar: import("./exchange2016calendar"),
-  exchangecalendar: import("./exchangecalendar"),
-  facetime: import("./facetime"),
-  sylapsvideo: import("./sylapsvideo"),
+  // example: () => import("./example"),
+  applecalendar: () => import("./applecalendar"),
+  caldavcalendar: () => import("./caldavcalendar"),
+  closecom: () => import("./closecom"),
+  dailyvideo: () => import("./dailyvideo"),
+  googlecalendar: () => import("./googlecalendar"),
+  googlevideo: () => import("./googlevideo"),
+  hubspot: () => import("./hubspot"),
+  huddle01video: () => import("./huddle01video"),
+  jitsivideo: () => import("./jitsivideo"),
+  larkcalendar: () => import("./larkcalendar"),
+  office365calendar: () => import("./office365calendar"),
+  office365video: () => import("./office365video"),
+  plausible: () => import("./plausible"),
+  salesforce: () => import("./salesforce"),
+  zohocrm: () => import("./zohocrm"),
+  sendgrid: () => import("./sendgrid"),
+  stripepayment: () => import("./stripepayment"),
+  tandemvideo: () => import("./tandemvideo"),
+  vital: () => import("./vital"),
+  zoomvideo: () => import("./zoomvideo"),
+  wipemycalother: () => import("./wipemycalother"),
+  giphy: () => import("./giphy"),
+  zapier: () => import("./zapier"),
+  exchange2013calendar: () => import("./exchange2013calendar"),
+  exchange2016calendar: () => import("./exchange2016calendar"),
+  exchangecalendar: () => import("./exchangecalendar"),
+  facetime: () => import("./facetime"),
+  sylapsvideo: () => import("./sylapsvideo"),
 };
 
 export default appStore;

🤖 Grapple PR auto-fix • major • Review this diff before applying

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (56 lines, 1 file) — review recommended)

The import() calls are executed eagerly at module evaluation time (when index.ts is first imported), not lazily on demand. The appStore object literal evaluates all its property initializers immediately, so every dynamic import() fires at once during module load. This provides no code-splitting or lazy-loading benefit — all app modules are still loaded upfront, just asynchronously. The refactor adds significant complexity (async propagation through ~12 files) without achieving the stated goal of lazy loading.

--- a/packages/app-store/index.ts
+++ b/packages/app-store/index.ts
@@ -1,31 +1,31 @@
 const appStore = {
   // example: () => import("./example"),
-  applecalendar: import("./applecalendar"),
-  caldavcalendar: import("./caldavcalendar"),
-  closecom: import("./closecom"),
-  dailyvideo: import("./dailyvideo"),
-  googlecalendar: import("./googlecalendar"),
-  googlevideo: import("./googlevideo"),
-  hubspot: import("./hubspot"),
-  huddle01video: import("./huddle01video"),
-  jitsivideo: import("./jitsivideo"),
-  larkcalendar: import("./larkcalendar"),
-  office365calendar: import("./office365calendar"),
-  office365video: import("./office365video"),
-  plausible: import("./plausible"),
-  salesforce: import("./salesforce"),
-  zohocrm: import("./zohocrm"),
-  sendgrid: import("./sendgrid"),
-  stripepayment: import("./stripepayment"),
-  tandemvideo: import("./tandemvideo"),
-  vital: import("./vital"),
-  zoomvideo: import("./zoomvideo"),
-  wipemycalother: import("./wipemycalother"),
-  giphy: import("./giphy"),
-  zapier: import("./zapier"),
-  exchange2013calendar: import("./exchange2013calendar"),
-  exchange2016calendar: import("./exchange2016calendar"),
-  exchangecalendar: import("./exchangecalendar"),
-  facetime: import("./facetime"),
-  sylapsvideo: import("./sylapsvideo"),
+  applecalendar: () => import("./applecalendar"),
+  caldavcalendar: () => import("./caldavcalendar"),
+  closecom: () => import("./closecom"),
+  dailyvideo: () => import("./dailyvideo"),
+  googlecalendar: () => import("./googlecalendar"),
+  googlevideo: () => import("./googlevideo"),
+  hubspot: () => import("./hubspot"),
+  huddle01video: () => import("./huddle01video"),
+  jitsivideo: () => import("./jitsivideo"),
+  larkcalendar: () => import("./larkcalendar"),
+  office365calendar: () => import("./office365calendar"),
+  office365video: () => import("./office365video"),
+  plausible: () => import("./plausible"),
+  salesforce: () => import("./salesforce"),
+  zohocrm: () => import("./zohocrm"),
+  sendgrid: () => import("./sendgrid"),
+  stripepayment: () => import("./stripepayment"),
+  tandemvideo: () => import("./tandemvideo"),
+  vital: () => import("./vital"),
+  zoomvideo: () => import("./zoomvideo"),
+  wipemycalother: () => import("./wipemycalother"),
+  giphy: () => import("./giphy"),
+  zapier: () => import("./zapier"),
+  exchange2013calendar: () => import("./exchange2013calendar"),
+  exchange2016calendar: () => import("./exchange2016calendar"),
+  exchangecalendar: () => import("./exchangecalendar"),
+  facetime: () => import("./facetime"),
+  sylapsvideo: () => import("./sylapsvideo"),
 };
 
 export default appStore;

🤖 Grapple PR auto-fix • major • Review this diff before applying

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — architecture agent (Small fix (2 lines, 1 file))

The sylapsvideo entry has been moved from its original alphabetical/grouped position to the very end of the object (line 30). While it IS present in the new code, the reordering combined with it being the last entry makes it easy to accidentally drop in future edits. More importantly, verify this was intentional — the original code had sylapsvideo between jitsivideo and larkcalendar, and the new code places it after facetime.

--- a/packages/app-store/index.ts
+++ b/packages/app-store/index.ts
@@ -8,8 +8,8 @@
   hubspot: import("./hubspot"),
   huddle01video: import("./huddle01video"),
   jitsivideo: import("./jitsivideo"),
+  sylapsvideo: import("./sylapsvideo"),
   larkcalendar: import("./larkcalendar"),
   office365calendar: import("./office365calendar"),
   office365video: import("./office365video"),
   plausible: import("./plausible"),
@@ -26,6 +26,5 @@
   exchange2013calendar: import("./exchange2013calendar"),
   exchange2016calendar: import("./exchange2016calendar"),
   exchangecalendar: import("./exchangecalendar"),
   facetime: import("./facetime"),
-  sylapsvideo: import("./sylapsvideo"),
 };

🤖 Grapple PR auto-fix • major • Review this diff before applying

@grapple-pr grapple-pr Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Lazy Loading Defeated (confidence: 100%)

All dynamic import() calls are executed eagerly at module load time since they are invoked inline in the object literal. Every app package will begin loading simultaneously when this module is first imported, defeating the stated goal of code splitting and lazy loading. The Promises are created immediately — not deferred until first access.

Evidence:

  • Each value in the appStore object is import('./somepackage'), which is evaluated immediately when the object is constructed
  • The intent specification states the goal is 'to enable code splitting and lazy loading of app store packages'
  • For true lazy loading, the imports should be wrapped in functions: applecalendar: () => import('./applecalendar') and called at point of use

Agent: architecture

@grapple-pr grapple-pr Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (58 lines, 1 file) — review recommended)

All dynamic import() calls are executed eagerly at module load time since they are invoked inline in the object literal. Every app package will begin loading simultaneously when this module is first imported, defeating the stated goal of code splitting and lazy loading. The Promises are created immediately — not deferred until first access.

--- a/packages/app-store/index.ts
+++ b/packages/app-store/index.ts
@@ -1,31 +1,31 @@
 const appStore = {
-  // example: import("./example"),
-  applecalendar: import("./applecalendar"),
-  caldavcalendar: import("./caldavcalendar"),
-  closecom: import("./closecom"),
-  dailyvideo: import("./dailyvideo"),
-  googlecalendar: import("./googlecalendar"),
-  googlevideo: import("./googlevideo"),
-  hubspot: import("./hubspot"),
-  huddle01video: import("./huddle01video"),
-  jitsivideo: import("./jitsivideo"),
-  larkcalendar: import("./larkcalendar"),
-  office365calendar: import("./office365calendar"),
-  office365video: import("./office365video"),
-  plausible: import("./plausible"),
-  salesforce: import("./salesforce"),
-  zohocrm: import("./zohocrm"),
-  sendgrid: import("./sendgrid"),
-  stripepayment: import("./stripepayment"),
-  tandemvideo: import("./tandemvideo"),
-  vital: import("./vital"),
-  zoomvideo: import("./zoomvideo"),
-  wipemycalother: import("./wipemycalother"),
-  giphy: import("./giphy"),
-  zapier: import("./zapier"),
-  exchange2013calendar: import("./exchange2013calendar"),
-  exchange2016calendar: import("./exchange2016calendar"),
-  exchangecalendar: import("./exchangecalendar"),
-  facetime: import("./facetime"),
-  sylapsvideo: import("./sylapsvideo"),
+  // example: () => import("./example"),
+  applecalendar: () => import("./applecalendar"),
+  caldavcalendar: () => import("./caldavcalendar"),
+  closecom: () => import("./closecom"),
+  dailyvideo: () => import("./dailyvideo"),
+  googlecalendar: () => import("./googlecalendar"),
+  googlevideo: () => import("./googlevideo"),
+  hubspot: () => import("./hubspot"),
+  huddle01video: () => import("./huddle01video"),
+  jitsivideo: () => import("./jitsivideo"),
+  larkcalendar: () => import("./larkcalendar"),
+  office365calendar: () => import("./office365calendar"),
+  office365video: () => import("./office365video"),
+  plausible: () => import("./plausible"),
+  salesforce: () => import("./salesforce"),
+  zohocrm: () => import("./zohocrm"),
+  sendgrid: () => import("./sendgrid"),
+  stripepayment: () => import("./stripepayment"),
+  tandemvideo: () => import("./tandemvideo"),
+  vital: () => import("./vital"),
+  zoomvideo: () => import("./zoomvideo"),
+  wipemycalother: () => import("./wipemycalother"),
+  giphy: () => import("./giphy"),
+  zapier: () => import("./zapier"),
+  exchange2013calendar: () => import("./exchange2013calendar"),
+  exchange2016calendar: () => import("./exchange2016calendar"),
+  exchangecalendar: () => import("./exchangecalendar"),
+  facetime: () => import("./facetime"),
+  sylapsvideo: () => import("./sylapsvideo"),
 };
 
 export default appStore;

🤖 Grapple PR auto-fix • major • Review this diff before applying

@grapple-pr grapple-pr Bot Apr 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — No lazy loading benefit (confidence: 99%)

The dynamic import() expressions are invoked immediately as object property initializers when the module is loaded. This means all app store modules are still loaded eagerly at startup — the imports execute immediately and produce Promises that resolve with the already-loading modules. The stated performance goal of lazy/async loading is not achieved; instead, this adds Promise resolution overhead to every call site without any code-splitting benefit.

Evidence:

  • Each property value is import('./...') which executes immediately when the object literal is evaluated
  • The appStore object is evaluated at module load time (top-level const)
  • True lazy loading would require wrapping in arrow functions: applecalendar: () => import('./applecalendar') and calling only when needed

Agent: logic

@grapple-pr grapple-pr Bot Apr 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Type safety (confidence: 95%)

The type of appStore values changed from module namespace objects to Promise. However, keyof typeof appStore and typeof appStore used in call sites (e.g., appStore[key as keyof typeof appStore]) now represent Promise-typed values. Any call site not included in this PR that accesses appStore without awaiting will receive a Promise object instead of a module, and since Promise objects are truthy and have no lib property, the 'lib' in app guard will return false — silently failing instead of throwing an error.

Evidence:

  • Multiple call sites use appStore[... as keyof typeof appStore]
  • The type of each value is now Promise instead of typeof import('./...')
  • Any uncovered call site will silently fail due to the 'lib' in app guard returning false for a Promise object

Agent: logic

@grapple-pr grapple-pr Bot Apr 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (58 lines, 1 file) — review recommended)

The dynamic import() expressions are invoked immediately as object property initializers when the module is loaded. This means all app store modules are still loaded eagerly at startup — the imports execute immediately and produce Promises that resolve with the already-loading modules. The stated performance goal of lazy/async loading is not achieved; instead, this adds Promise resolution overhead to every call site without any code-splitting benefit.

--- a/packages/app-store/index.ts
+++ b/packages/app-store/index.ts
@@ -1,31 +1,31 @@
 const appStore = {
-  // example: import("./example"),
-  applecalendar: import("./applecalendar"),
-  caldavcalendar: import("./caldavcalendar"),
-  closecom: import("./closecom"),
-  dailyvideo: import("./dailyvideo"),
-  googlecalendar: import("./googlecalendar"),
-  googlevideo: import("./googlevideo"),
-  hubspot: import("./hubspot"),
-  huddle01video: import("./huddle01video"),
-  jitsivideo: import("./jitsivideo"),
-  larkcalendar: import("./larkcalendar"),
-  office365calendar: import("./office365calendar"),
-  office365video: import("./office365video"),
-  plausible: import("./plausible"),
-  salesforce: import("./salesforce"),
-  zohocrm: import("./zohocrm"),
-  sendgrid: import("./sendgrid"),
-  stripepayment: import("./stripepayment"),
-  tandemvideo: import("./tandemvideo"),
-  vital: import("./vital"),
-  zoomvideo: import("./zoomvideo"),
-  wipemycalother: import("./wipemycalother"),
-  giphy: import("./giphy"),
-  zapier: import("./zapier"),
-  exchange2013calendar: import("./exchange2013calendar"),
-  exchange2016calendar: import("./exchange2016calendar"),
-  exchangecalendar: import("./exchangecalendar"),
-  facetime: import("./facetime"),
-  sylapsvideo: import("./sylapsvideo"),
+  // example: () => import("./example"),
+  applecalendar: () => import("./applecalendar"),
+  caldavcalendar: () => import("./caldavcalendar"),
+  closecom: () => import("./closecom"),
+  dailyvideo: () => import("./dailyvideo"),
+  googlecalendar: () => import("./googlecalendar"),
+  googlevideo: () => import("./googlevideo"),
+  hubspot: () => import("./hubspot"),
+  huddle01video: () => import("./huddle01video"),
+  jitsivideo: () => import("./jitsivideo"),
+  larkcalendar: () => import("./larkcalendar"),
+  office365calendar: () => import("./office365calendar"),
+  office365video: () => import("./office365video"),
+  plausible: () => import("./plausible"),
+  salesforce: () => import("./salesforce"),
+  zohocrm: () => import("./zohocrm"),
+  sendgrid: () => import("./sendgrid"),
+  stripepayment: () => import("./stripepayment"),
+  tandemvideo: () => import("./tandemvideo"),
+  vital: () => import("./vital"),
+  zoomvideo: () => import("./zoomvideo"),
+  wipemycalother: () => import("./wipemycalother"),
+  giphy: () => import("./giphy"),
+  zapier: () => import("./zapier"),
+  exchange2013calendar: () => import("./exchange2013calendar"),
+  exchange2016calendar: () => import("./exchange2016calendar"),
+  exchangecalendar: () => import("./exchangecalendar"),
+  facetime: () => import("./facetime"),
+  sylapsvideo: () => import("./sylapsvideo"),
 };
 
 export default appStore;

🤖 Grapple PR auto-fix • major • Review this diff before applying

@grapple-pr grapple-pr Bot Apr 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (18 lines, 1 file) — review recommended)

The type of appStore values changed from module namespace objects to Promise<typeof import(...)>. However, keyof typeof appStore and typeof appStore used in call sites (e.g., appStore[key as keyof typeof appStore]) now represent Promise-typed values. Any call site not included in this PR that accesses appStore without awaiting will receive a Promise object instead of a module, and since Promise objects are truthy and have no lib property, the 'lib' in app guard will return false — silently failing instead of throwing an error.

--- a/packages/app-store/index.ts
+++ b/packages/app-store/index.ts
@@ -1,33 +1,51 @@
 const appStore = {
   // example: import("./example"),
   applecalendar: import("./applecalendar"),
   caldavcalendar: import("./caldavcalendar"),
   closecom: import("./closecom"),
   dailyvideo: import("./dailyvideo"),
   googlecalendar: import("./googlecalendar"),
   googlevideo: import("./googlevideo"),
   hubspot: import("./hubspot"),
   huddle01video: import("./huddle01video"),
   jitsivideo: import("./jitsivideo"),
   larkcalendar: import("./larkcalendar"),
   office365calendar: import("./office365calendar"),
   office365video: import("./office365video"),
   plausible: import("./plausible"),
   salesforce: import("./salesforce"),
   zohocrm: import("./zohocrm"),
   sendgrid: import("./sendgrid"),
   stripepayment: import("./stripepayment"),
   tandemvideo: import("./tandemvideo"),
   vital: import("./vital"),
   zoomvideo: import("./zoomvideo"),
   wipemycalother: import("./wipemycalother"),
   giphy: import("./giphy"),
   zapier: import("./zapier"),
   exchange2013calendar: import("./exchange2013calendar"),
   exchange2016calendar: import("./exchange2016calendar"),
   exchangecalendar: import("./exchangecalendar"),
   facetime: import("./facetime"),
   sylapsvideo: import("./sylapsvideo"),
 };
 
+export type AppStoreModules = typeof appStore;
+export type AppKeys = keyof AppStoreModules;
+
+/**
+ * Type-safe async accessor for appStore entries.
+ * Always use this instead of `appStore[key]` directly to ensure the Promise is
+ * awaited before use. Direct indexing yields a Promise object — accessing
+ * properties like `lib` on it will silently return undefined rather than
+ * throwing, because Promise objects are truthy and `'lib' in promise === false`.
+ *
+ * @example
+ *   const app = await getAppStore("zoomvideo");
+ *   if (app && "lib" in app) { ... }
+ */
+export async function getAppStore<T extends AppKeys>(name: T): Promise<Awaited<AppStoreModules[T]>> {
+  return appStore[name];
+}
+
 export default appStore;

🤖 Grapple PR auto-fix • minor • Review this diff before applying


@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 CRITICAL — Unnecessary work (confidence: 100%)

All dynamic import() calls are evaluated eagerly at module load time, not lazily. The entire point of dynamic imports for code-splitting is that they are called conditionally or on-demand. Placing all import() calls as top-level object property initializers means every app module is fetched immediately when this file is first imported — identical behavior to static imports, but with added Promise overhead on every access. No lazy loading or code-splitting benefit is achieved.

Evidence:

  • Each import('./appname') expression is evaluated when the object literal is constructed, which happens at module initialization time.
  • Every consumer must now await each appStore entry even though all modules were already loaded at startup.
  • The stated goal of 'lazy loading of app store packages' is not achieved — this pattern only helps with circular dependency resolution, not deferred loading.

Agent: performance

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — performance agent (Larger fix (58 lines, 1 file) — review recommended)

All dynamic import() calls are evaluated eagerly at module load time, not lazily. The entire point of dynamic imports for code-splitting is that they are called conditionally or on-demand. Placing all import() calls as top-level object property initializers means every app module is fetched immediately when this file is first imported — identical behavior to static imports, but with added Promise overhead on every access. No lazy loading or code-splitting benefit is achieved.

--- a/packages/app-store/index.ts
+++ b/packages/app-store/index.ts
@@ -1,33 +1,33 @@
 const appStore = {
-  // example: import("./example"),
-  applecalendar: import("./applecalendar"),
-  caldavcalendar: import("./caldavcalendar"),
-  closecom: import("./closecom"),
-  dailyvideo: import("./dailyvideo"),
-  googlecalendar: import("./googlecalendar"),
-  googlevideo: import("./googlevideo"),
-  hubspot: import("./hubspot"),
-  huddle01video: import("./huddle01video"),
-  jitsivideo: import("./jitsivideo"),
-  larkcalendar: import("./larkcalendar"),
-  office365calendar: import("./office365calendar"),
-  office365video: import("./office365video"),
-  plausible: import("./plausible"),
-  salesforce: import("./salesforce"),
-  zohocrm: import("./zohocrm"),
-  sendgrid: import("./sendgrid"),
-  stripepayment: import("./stripepayment"),
-  tandemvideo: import("./tandemvideo"),
-  vital: import("./vital"),
-  zoomvideo: import("./zoomvideo"),
-  wipemycalother: import("./wipemycalother"),
-  giphy: import("./giphy"),
-  zapier: import("./zapier"),
-  exchange2013calendar: import("./exchange2013calendar"),
-  exchange2016calendar: import("./exchange2016calendar"),
-  exchangecalendar: import("./exchangecalendar"),
-  facetime: import("./facetime"),
-  sylapsvideo: import("./sylapsvideo"),
+  // example: () => import("./example"),
+  applecalendar: () => import("./applecalendar"),
+  caldavcalendar: () => import("./caldavcalendar"),
+  closecom: () => import("./closecom"),
+  dailyvideo: () => import("./dailyvideo"),
+  googlecalendar: () => import("./googlecalendar"),
+  googlevideo: () => import("./googlevideo"),
+  hubspot: () => import("./hubspot"),
+  huddle01video: () => import("./huddle01video"),
+  jitsivideo: () => import("./jitsivideo"),
+  larkcalendar: () => import("./larkcalendar"),
+  office365calendar: () => import("./office365calendar"),
+  office365video: () => import("./office365video"),
+  plausible: () => import("./plausible"),
+  salesforce: () => import("./salesforce"),
+  zohocrm: () => import("./zohocrm"),
+  sendgrid: () => import("./sendgrid"),
+  stripepayment: () => import("./stripepayment"),
+  tandemvideo: () => import("./tandemvideo"),
+  vital: () => import("./vital"),
+  zoomvideo: () => import("./zoomvideo"),
+  wipemycalother: () => import("./wipemycalother"),
+  giphy: () => import("./giphy"),
+  zapier: () => import("./zapier"),
+  exchange2013calendar: () => import("./exchange2013calendar"),
+  exchange2016calendar: () => import("./exchange2016calendar"),
+  exchangecalendar: () => import("./exchangecalendar"),
+  facetime: () => import("./facetime"),
+  sylapsvideo: () => import("./sylapsvideo"),
 };
 
 export default appStore;

🤖 Grapple PR auto-fix • critical • Review this diff before applying

export default appStore;
4 changes: 2 additions & 2 deletions packages/app-store/vital/lib/reschedule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,10 @@ const Reschedule = async (bookingUid: string, cancellationReason: string) => {
(ref) => !!credentialsMap.get(ref.type)
);
try {
bookingRefsFiltered.forEach((bookingRef) => {
bookingRefsFiltered.forEach(async (bookingRef) => {
if (bookingRef.uid) {
if (bookingRef.type.endsWith("_calendar")) {
const calendar = getCalendar(credentialsMap.get(bookingRef.type));
const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
} else if (bookingRef.type.endsWith("_video")) {
return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 CRITICAL — Concurrency issues (confidence: 100%)

An async callback is passed to forEach(). forEach() does not await async callbacks — it fires each async function and discards the returned Promise. This means calendar.deleteEvent() and deleteMeeting() calls are fire-and-forget: errors are silently swallowed, the surrounding try/catch cannot catch rejections, and the function may return before deletions complete.

Evidence:

  • Array.prototype.forEach does not handle Promise return values from callbacks.
  • The async keyword on the callback creates a Promise that is never awaited by forEach.
  • If getCalendar() rejects or deleteEvent() fails, the error is lost with no logging or retry.
  • Same pattern existed before but was synchronous — the async conversion makes this actively dangerous.

Agent: performance

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — performance agent (Larger fix (20 lines, 1 file) — review recommended)

An async callback is passed to forEach(). forEach() does not await async callbacks — it fires each async function and discards the returned Promise. This means calendar.deleteEvent() and deleteMeeting() calls are fire-and-forget: errors are silently swallowed, the surrounding try/catch cannot catch rejections, and the function may return before deletions complete.

--- a/packages/app-store/vital/lib/reschedule.ts
+++ b/packages/app-store/vital/lib/reschedule.ts
@@ -122,13 +122,15 @@ const Reschedule = async (bookingUid: string, cancellationReason: string) => {
       (ref) => !!credentialsMap.get(ref.type)
     );
     try {
-      bookingRefsFiltered.forEach(async (bookingRef) => {
-        if (bookingRef.uid) {
-          if (bookingRef.type.endsWith("_calendar")) {
-            const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
-            return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
-          } else if (bookingRef.type.endsWith("_video")) {
-            return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
+      await Promise.all(
+        bookingRefsFiltered.map(async (bookingRef) => {
+          if (bookingRef.uid) {
+            if (bookingRef.type.endsWith("_calendar")) {
+              const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
+              await calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
+            } else if (bookingRef.type.endsWith("_video")) {
+              await deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
+            }
           }
-        }
-      });
+        })
+      );
     } catch (error) {
       if (error instanceof Error) {
         logger.error(error.message);

🤖 Grapple PR auto-fix • critical • Review this diff before applying

Expand Down
4 changes: 2 additions & 2 deletions packages/app-store/wipemycalother/lib/reschedule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,10 @@ const Reschedule = async (bookingUid: string, cancellationReason: string) => {
(ref) => !!credentialsMap.get(ref.type)
);
try {
bookingRefsFiltered.forEach((bookingRef) => {
bookingRefsFiltered.forEach(async (bookingRef) => {
if (bookingRef.uid) {
if (bookingRef.type.endsWith("_calendar")) {
const calendar = getCalendar(credentialsMap.get(bookingRef.type));
const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
} else if (bookingRef.type.endsWith("_video")) {
return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 CRITICAL — Concurrency issues (confidence: 100%)

Same async-in-forEach anti-pattern as vital/lib/reschedule.ts. Calendar delete operations are fire-and-forget with no error handling and no guarantee of completion before the function returns.

Evidence:

  • Array.prototype.forEach discards Promise return values.
  • Exceptions thrown inside the async callback will become unhandled rejections.
  • Rescheduling may complete while calendar events are still pending deletion.

Agent: performance

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — performance agent (Larger fix (20 lines, 1 file) — review recommended)

Same async-in-forEach anti-pattern as vital/lib/reschedule.ts. Calendar delete operations are fire-and-forget with no error handling and no guarantee of completion before the function returns.

--- a/packages/app-store/wipemycalother/lib/reschedule.ts
+++ b/packages/app-store/wipemycalother/lib/reschedule.ts
@@ -122,14 +122,16 @@ const Reschedule = async (bookingUid: string, cancellationReason: string) => {
       (ref) => !!credentialsMap.get(ref.type)
     );
     try {
-      bookingRefsFiltered.forEach(async (bookingRef) => {
-        if (bookingRef.uid) {
-          if (bookingRef.type.endsWith("_calendar")) {
-            const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
-            return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
-          } else if (bookingRef.type.endsWith("_video")) {
-            return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
+      await Promise.all(
+        bookingRefsFiltered.map(async (bookingRef) => {
+          if (bookingRef.uid) {
+            if (bookingRef.type.endsWith("_calendar")) {
+              const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
+              return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
+            } else if (bookingRef.type.endsWith("_video")) {
+              return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
+            }
           }
-        }
-      });
+        })
+      );
     } catch (error) {
       if (error instanceof Error) {
         logger.error(error.message);

🤖 Grapple PR auto-fix • critical • Review this diff before applying

Expand Down
15 changes: 8 additions & 7 deletions packages/core/CalendarManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const getCalendarCredentials = (credentials: Array<CredentialPayload>) =>
const calendar = getCalendar(credential);
return app.variant === "calendar" ? [{ integration: app, credential, calendar }] : [];

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Unresolved Promise in return type (confidence: 100%)

In getCalendarCredentials(), the calendar property in the returned objects is now a Promise (since getCalendar() is now async), but the function is not async and does not await the result. This means every consumer of getCalendarCredentials() receives objects where calendar is a Promise, not a Calendar instance. While getConnectedCalendars() correctly awaits item.calendar on line 47, and getCachedResults() was refactored to use Promise.all, any other current or future caller that destructures calendar directly without awaiting will receive a Promise object, which is truthy, causing silent logic failures (e.g., if (calendar) would always be true even if the resolved value is null).

Evidence:

  • Line 28: const calendar = getCalendar(credential); — getCalendar is now async, returns Promise
  • The return type of getCalendarCredentials implicitly changes the calendar field from Calendar | null to Promise
  • This is a subtle API contract change that TypeScript may not flag at all call sites depending on type inference
  • Intent specification notes: 'any consumer that destructures calendar directly without awaiting will receive a Promise object instead of a Calendar instance, causing silent failures'

Agent: logic

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (20 lines, 1 file) — review recommended)

In getCalendarCredentials(), the calendar property in the returned objects is now a Promise<Calendar | null> (since getCalendar() is now async), but the function is not async and does not await the result. This means every consumer of getCalendarCredentials() receives objects where calendar is a Promise, not a Calendar instance. While getConnectedCalendars() correctly awaits item.calendar on line 47, and getCachedResults() was refactored to use Promise.all, any other current or future caller that destructures calendar directly without awaiting will receive a Promise object, which is truthy, causing silent logic failures (e.g., if (calendar) would always be true even if the resolved value is null).

--- a/packages/core/CalendarManager.ts
+++ b/packages/core/CalendarManager.ts
@@ -23,14 +23,17 @@ const log = logger.getChildLogger({ prefix: ["CalendarManager"] });
 
-export const getCalendarCredentials = (credentials: Array<CredentialPayload>) => {
-  const calendarCredentials = getApps(credentials)
+export const getCalendarCredentials = async (credentials: Array<CredentialPayload>) => {
+  const calendarCredentials = await Promise.all(getApps(credentials)
     .filter((app) => app.type.endsWith("_calendar"))
-    .flatMap((app) => {
-      const credentials = app.credentials.flatMap((credential) => {
-        const calendar = getCalendar(credential);
-        return app.variant === "calendar" ? [{ integration: app, credential, calendar }] : [];
-      });
+    .flatMap((app) => {
+      return app.credentials
+        .filter(() => app.variant === "calendar")
+        .map(async (credential) => {
+          const calendar = await getCalendar(credential);
+          return { integration: app, credential, calendar };
+        });
+    }));
 
-      return credentials.length ? credentials : [];
-    });
-
   return calendarCredentials;
 };
 
@@ -38,7 +41,7 @@ export const getConnectedCalendars = async (
   calendarCredentials: ReturnType<typeof getCalendarCredentials>,

🤖 Grapple PR auto-fix • major • Review this diff before applying

@grapple-pr grapple-pr Bot Apr 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Unresolved Promise stored in object (confidence: 100%)

getCalendar() is now async and returns Promise, but in getCalendarCredentials() the .map() callback is not async and does not await the result. This means item.calendar is a Promise, not Calendar | null. While getConnectedCalendars() at line 47 does await item.calendar, any other consumer of getCalendarCredentials() that accesses .calendar without awaiting will receive a Promise object that is always truthy, bypassing null checks.

Evidence:

  • Line 28: const calendar = getCalendar(credential); — getCalendar is now async, so this returns a Promise
  • Line 29: return app.variant === 'calendar' ? [{ integration: app, credential, calendar }] : []; — calendar is a Promise here
  • The type system may not catch this if the return type is not explicitly typed, since Promise is a valid object
  • Any code that does if (!item.calendar) will always be false since a Promise object is truthy

Agent: logic

@grapple-pr grapple-pr Bot Apr 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (16 lines, 1 file) — review recommended)

getCalendar() is now async and returns Promise<Calendar | null>, but in getCalendarCredentials() the .map() callback is not async and does not await the result. This means item.calendar is a Promise<Calendar | null>, not Calendar | null. While getConnectedCalendars() at line 47 does await item.calendar, any other consumer of getCalendarCredentials() that accesses .calendar without awaiting will receive a Promise object that is always truthy, bypassing null checks.

--- a/packages/core/CalendarManager.ts
+++ b/packages/core/CalendarManager.ts
@@ -26,7 +26,7 @@ export const getCalendarCredentials = (credentials: Array<CredentialPayload>) =>
       const credentials = app.credentials.flatMap((credential) => {
         const calendar = getCalendar(credential);
-        return app.variant === "calendar" ? [{ integration: app, credential, calendar }] : [];
+        return app.variant === "calendar" ? [{ integration: app, credential, calendar: calendar as Promise<Calendar | null> }] : [];
       });
 
       return credentials.length ? credentials : [];
@@ -44,8 +44,8 @@ export const getConnectedCalendars = async (
   const connectedCalendars = await Promise.all(
     calendarCredentials.map(async (item) => {
       try {
-        const { integration, credential } = item;
-        const calendar = await item.calendar;
+        const { integration, credential } = item;
+        const calendar = await item.calendar;
         // Don't leak credentials to the client
         const credentialId = credential.id;
         if (!calendar) {
@@ -139,7 +139,7 @@ export const getCachedResults = async (
   selectedCalendars: SelectedCalendar[]
 ): Promise<EventBusyDate[][]> => {
   const calendarCredentials = withCredentials.filter((credential) => credential.type.endsWith("_calendar"));
-  const calendars = calendarCredentials.map((credential) => getCalendar(credential));
+  const calendars = await Promise.all(calendarCredentials.map((credential) => getCalendar(credential)));
   performance.mark("getBusyCalendarTimesStart");
   const results = calendars.map(async (c, i) => {
     /** Filter out nulls */
@@ -230,7 +230,7 @@ export const createEvent = async (
   calEvent: CalendarEvent
 ): Promise<EventResult<NewCalendarEventType>> => {
   const uid: string = getUid(calEvent);
-  const calendar = getCalendar(credential);
+  const calendar = await getCalendar(credential);
   let success = true;
   let calError: string | undefined = undefined;
 
@@ -281,7 +281,7 @@ export const updateEvent = async (
   externalCalendarId: string | null
 ): Promise<EventResult<NewCalendarEventType>> => {
   const uid = getUid(calEvent);
-  const calendar = getCalendar(credential);
+  const calendar = await getCalendar(credential);
   let success = false;
   let calError: string | undefined = undefined;
   let calWarnings: string[] | undefined = [];
@@ -327,12 +327,12 @@ export const updateEvent = async (
   };
 };
 
-export const deleteEvent = (
+export const deleteEvent = async (
   credential: CredentialPayload,
   uid: string,
   event: CalendarEvent
 ): Promise<unknown> => {
-  const calendar = getCalendar(credential);
+  const calendar = await getCalendar(credential);
   if (calendar) {
     return calendar.deleteEvent(uid, event);
   }

🤖 Grapple PR auto-fix • major • Review this diff before applying

});

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — API contract / hidden Promise in return type (confidence: 100%)

The getCalendarCredentials function returns objects where the calendar property is now a Promise (since getCalendar() is now async), but the function signature and return type are not updated to reflect this. The function itself remains synchronous, so callers receive { integration, credential, calendar: Promise }. While getConnectedCalendars correctly awaits item.calendar (line 46), any other current or future consumer of getCalendarCredentials that destructures calendar directly will get a Promise object instead of a Calendar instance, causing silent type-level failures (the truthiness check if (!calendar) will always pass since a Promise is truthy).

Evidence:

  • Line 28: const calendar = getCalendar(credential) — getCalendar is now async, so calendar is a Promise
  • The return type of getCalendarCredentials is not explicitly typed to reflect Promise in the calendar field
  • Line 46 in getConnectedCalendars correctly awaits: const calendar = await item.calendar — but this is fragile; other callers may not
  • TypeScript may infer the type correctly, but the implicit Promise wrapping is a subtle API contract change that should be explicit

Agent: architecture

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (29 lines, 1 file) — review recommended)

The getCalendarCredentials function returns objects where the calendar property is now a Promise<Calendar | null> (since getCalendar() is now async), but the function signature and return type are not updated to reflect this. The function itself remains synchronous, so callers receive { integration, credential, calendar: Promise<Calendar | null> }. While getConnectedCalendars correctly awaits item.calendar (line 46), any other current or future consumer of getCalendarCredentials that destructures calendar directly will get a Promise object instead of a Calendar instance, causing silent type-level failures (the truthiness check if (!calendar) will always pass since a Promise is truthy).

--- a/packages/core/CalendarManager.ts
+++ b/packages/core/CalendarManager.ts
@@ -23,16 +23,16 @@ const log = logger.getChildLogger({ prefix: ["CalendarManager"] });
 
-export const getCalendarCredentials = (credentials: Array<CredentialPayload>) => {
-  const calendarCredentials = getApps(credentials)
+export const getCalendarCredentials = async (credentials: Array<CredentialPayload>) => {
+  const calendarCredentials = await Promise.all(getApps(credentials)
     .filter((app) => app.type.endsWith("_calendar"))
     .flatMap((app) => {
-      const credentials = app.credentials.flatMap((credential) => {
-        const calendar = getCalendar(credential);
-        return app.variant === "calendar" ? [{ integration: app, credential, calendar }] : [];
+      const credentials = app.credentials.flatMap((credential) => {
+        return app.variant === "calendar" ? [{ integration: app, credential }] : [];
       });
 
       return credentials.length ? credentials : [];
-    });
+    })
+    .map(async (item) => ({
+      ...item,
+      calendar: await getCalendar(item.credential),
+    })));
 
   return calendarCredentials;
 };
@@ -43,8 +43,8 @@ export const getConnectedCalendars = async (
   const connectedCalendars = await Promise.all(
     calendarCredentials.map(async (item) => {
       try {
-        const { integration, credential } = item;
-        const calendar = await item.calendar;
+        const { calendar, integration, credential } = item;
+
         // Don't leak credentials to the client
         const credentialId = credential.id;
         if (!calendar) {
@@ -138,7 +138,7 @@ export const getCachedResults = async (
   selectedCalendars: SelectedCalendar[]
 ): Promise<EventBusyDate[][]> => {
   const calendarCredentials = withCredentials.filter((credential) => credential.type.endsWith("_calendar"));
-  const calendars = await Promise.all(calendarCredentials.map((credential) => getCalendar(credential)));
+  const calendars = await Promise.all(calendarCredentials.map((credential) => getCalendar(credential)));
   performance.mark("getBusyCalendarTimesStart");
   const results = calendars.map(async (c, i) => {
     /** Filter out nulls */
@@ -229,7 +229,7 @@ export const createEvent = async (
   calEvent: CalendarEvent
 ): Promise<EventResult<NewCalendarEventType>> => {
   const uid: string = getUid(calEvent);
-  const calendar = await getCalendar(credential);
+  const calendar = await getCalendar(credential);
   let success = true;
   let calError: string | undefined = undefined;
 
@@ -281,7 +281,7 @@ export const updateEvent = async (
   externalCalendarId: string | null
 ): Promise<EventResult<NewCalendarEventType>> => {
   const uid = getUid(calEvent);
-  const calendar = await getCalendar(credential);
+  const calendar = await getCalendar(credential);
   let success = false;
   let calError: string | undefined = undefined;
   let calWarnings: string[] | undefined = [];
@@ -327,7 +327,7 @@ export const updateEvent = async (
 
-export const deleteEvent = async (
+export const deleteEvent = async (
   credential: CredentialPayload,
   uid: string,
   event: CalendarEvent
 ): Promise<unknown> => {
-  const calendar = await getCalendar(credential);
+  const calendar = await getCalendar(credential);
   if (calendar) {
     return calendar.deleteEvent(uid, event);
   }

🤖 Grapple PR auto-fix • major • Review this diff before applying


@grapple-pr grapple-pr Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Unawaited Promise in Type Contract (confidence: 100%)

In getCalendarCredentials(), the calendar field is now assigned the return value of getCalendar(credential) which is a Promise (since getCalendar is now async). However, the function is not async and does not await this value. The returned objects have calendar as a Promise, not a resolved Calendar. Any consumer that destructures calendar and checks if (!calendar) will get a truthy Promise object, bypassing null checks and leading to runtime errors when calling calendar methods on a Promise.

Evidence:

  • Line 28: const calendar = getCalendar(credential); — getCalendar is now async (returns Promise)
  • getCalendarCredentials is synchronous — it does not await the result
  • Line 47 in getConnectedCalendars correctly does const calendar = await item.calendar; which works around this, but getCachedResults on line 141 does await Promise.all(calendarCredentials.map((credential) => getCalendar(credential))) — a separate call rather than using getCalendarCredentials
  • The type contract is implicitly broken: consumers expect calendar: Calendar | null but receive calendar: Promise

Agent: architecture

@grapple-pr grapple-pr Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (26 lines, 1 file) — review recommended)

In getCalendarCredentials(), the calendar field is now assigned the return value of getCalendar(credential) which is a Promise<Calendar | null> (since getCalendar is now async). However, the function is not async and does not await this value. The returned objects have calendar as a Promise, not a resolved Calendar. Any consumer that destructures calendar and checks if (!calendar) will get a truthy Promise object, bypassing null checks and leading to runtime errors when calling calendar methods on a Promise.

--- a/packages/core/CalendarManager.ts
+++ b/packages/core/CalendarManager.ts
@@ -23,15 +23,18 @@ const log = logger.getChildLogger({ prefix: ["CalendarManager"] });
 
-export const getCalendarCredentials = (credentials: Array<CredentialPayload>) => {
-  const calendarCredentials = getApps(credentials)
+export const getCalendarCredentials = async (credentials: Array<CredentialPayload>) => {
+  const calendarCredentials = await Promise.all(
+    getApps(credentials)
     .filter((app) => app.type.endsWith("_calendar"))
-    .flatMap((app) => {
-      const credentials = app.credentials.flatMap((credential) => {
-        const calendar = getCalendar(credential);
-        return app.variant === "calendar" ? [{ integration: app, credential, calendar }] : [];
-      });
-
+    .flatMap((app) =>
+      app.credentials.flatMap((credential) =>
+        app.variant === "calendar"
+          ? [getCalendar(credential).then((calendar) => ({ integration: app, credential, calendar }))]
+          : []
+      )
+    )
+  ).then((results) =>
+    results.filter((item): item is Exclude<typeof item, { calendar: null }> & { calendar: NonNullable<typeof item.calendar> } | typeof item =>
+      true
+    )
+  );
 
-      return credentials.length ? credentials : [];
-    });
-
   return calendarCredentials;
 };

🤖 Grapple PR auto-fix • major • Review this diff before applying

return credentials.length ? credentials : [];
});

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 INFO — Code patterns (confidence: 96%)

getCalendarCredentials() returns an array with a calendar property that is now a Promise, but this function is synchronous and doesn't await the Promise. Callers that destructure calendar directly will receive a Promise object instead of a Calendar instance.

Evidence:

  • Line 27: const calendar = getCalendar(credential); — getCalendar now returns a Promise but is called without await
  • Line 28: return app.variant === 'calendar' ? [{ integration: app, credential, calendar }] : []; — calendar is a Promise, not a Calendar
  • getCalendarCredentials is synchronous (no async keyword) so cannot await
  • getConnectedCalendars correctly awaits on line 45, but other callers may not

Agent: style


Expand All @@ -43,8 +44,8 @@ export const getConnectedCalendars = async (
const connectedCalendars = await Promise.all(
calendarCredentials.map(async (item) => {
try {
const { calendar, integration, credential } = item;

const { integration, credential } = item;
const calendar = await item.calendar;
// Don't leak credentials to the client
const credentialId = credential.id;
if (!calendar) {
Expand Down Expand Up @@ -138,7 +139,7 @@ export const getCachedResults = async (
selectedCalendars: SelectedCalendar[]
): Promise<EventBusyDate[][]> => {
const calendarCredentials = withCredentials.filter((credential) => credential.type.endsWith("_calendar"));
const calendars = calendarCredentials.map((credential) => getCalendar(credential));
const calendars = await Promise.all(calendarCredentials.map((credential) => getCalendar(credential)));
performance.mark("getBusyCalendarTimesStart");
const results = calendars.map(async (c, i) => {
/** Filter out nulls */

@grapple-pr grapple-pr Bot Apr 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Null/undefined guard after await (confidence: 94%)

getCachedResults resolves calendars via Promise.all(map(getCalendar)), but getCalendar can return null. The subsequent results map at line 142 iterates over calendars and checks if (!c) — however, the null calendars still produce entries in the results array. The existing code already handled this, but it's worth verifying that the Promise.all resolution doesn't change behavior when some promises resolve to null.

Evidence:

  • Line 141: const calendars = await Promise.all(calendarCredentials.map((credential) => getCalendar(credential)));
  • getCalendar can return null for invalid/missing credentials
  • Line 142-143: The existing null check if (!c) should still work since await null === null

Agent: logic

@grapple-pr grapple-pr Bot Apr 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))

getCachedResults resolves calendars via Promise.all(map(getCalendar)), but getCalendar can return null. The subsequent results map at line 142 iterates over calendars and checks if (!c) — however, the null calendars still produce entries in the results array. The existing code already handled this, but it's worth verifying that the Promise.all resolution doesn't change behavior when some promises resolve to null.

Suggested change
/** Filter out nulls */
const calendars = await Promise.all(calendarCredentials.map((credential) => getCalendar(credential)));

🤖 Grapple PR auto-fix • major • confidence: 94%

Expand Down Expand Up @@ -229,7 +230,7 @@ export const createEvent = async (
calEvent: CalendarEvent
): Promise<EventResult<NewCalendarEventType>> => {
const uid: string = getUid(calEvent);
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
let success = true;
let calError: string | undefined = undefined;

Expand Down Expand Up @@ -280,7 +281,7 @@ export const updateEvent = async (
externalCalendarId: string | null
): Promise<EventResult<NewCalendarEventType>> => {
const uid = getUid(calEvent);
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
let success = false;
let calError: string | undefined = undefined;
let calWarnings: string[] | undefined = [];
Expand Down Expand Up @@ -326,12 +327,12 @@ export const updateEvent = async (
};
};

export const deleteEvent = (
export const deleteEvent = async (
credential: CredentialPayload,
uid: string,
event: CalendarEvent
): Promise<unknown> => {
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
if (calendar) {
return calendar.deleteEvent(uid, event);
}
Expand Down
3 changes: 1 addition & 2 deletions packages/core/EventManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,8 +485,7 @@ export default class EventManager {
id: oldCalendarEvent.credentialId,
},
});
const calendar = getCalendar(calendarCredential);

const calendar = await getCalendar(calendarCredential);
await calendar?.deleteEvent(oldCalendarEvent.uid, event, oldCalendarEvent.externalCalendarId);
}
}
Expand Down
36 changes: 20 additions & 16 deletions packages/core/videoClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,25 @@ const log = logger.getChildLogger({ prefix: ["[lib] videoClient"] });
const translator = short();

// factory
const getVideoAdapters = (withCredentials: CredentialPayload[]): VideoApiAdapter[] =>
withCredentials.reduce<VideoApiAdapter[]>((acc, cred) => {
const getVideoAdapters = async (withCredentials: CredentialPayload[]): Promise<VideoApiAdapter[]> => {
const videoAdapters: VideoApiAdapter[] = [];

for (const cred of withCredentials) {
const appName = cred.type.split("_").join(""); // Transform `zoom_video` to `zoomvideo`;
const app = appStore[appName as keyof typeof appStore];
const app = await appStore[appName as keyof typeof appStore];

if (app && "lib" in app && "VideoApiAdapter" in app.lib) {
const makeVideoApiAdapter = app.lib.VideoApiAdapter as VideoApiAdapterFactory;
const videoAdapter = makeVideoApiAdapter(cred);
acc.push(videoAdapter);
return acc;
videoAdapters.push(videoAdapter);
}
return acc;
}, []);
}

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Sequential awaits where parallelism is possible (confidence: 100%)

The refactored getVideoAdapters uses a sequential for...of loop with await for each credential. Since the dynamic imports are independent of each other, they could be resolved in parallel using Promise.all, which would be faster when multiple video credentials exist.

Evidence:

  • Lines 24-32: Sequential for (const cred of withCredentials) { const app = await appStore[...]; ... }
  • Each iteration waits for the previous import to resolve before starting the next
  • The original reduce pattern was synchronous and processed all credentials without delay

Agent: architecture

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (22 lines, 1 file) — review recommended)

The refactored getVideoAdapters uses a sequential for...of loop with await for each credential. Since the dynamic imports are independent of each other, they could be resolved in parallel using Promise.all, which would be faster when multiple video credentials exist.

--- a/packages/core/videoClient.ts
+++ b/packages/core/videoClient.ts
@@ -21,18 +21,16 @@ const translator = short();
 
 // factory
 const getVideoAdapters = async (withCredentials: CredentialPayload[]): Promise<VideoApiAdapter[]> => {
-  const videoAdapters: VideoApiAdapter[] = [];
-
-  for (const cred of withCredentials) {
+  const adapters = await Promise.all(
+    withCredentials.map(async (cred) => {
       const appName = cred.type.split("_").join(""); // Transform `zoom_video` to `zoomvideo`;
-    const app = await appStore[appName as keyof typeof appStore];
-
-    if (app && "lib" in app && "VideoApiAdapter" in app.lib) {
+      const app = await appStore[appName as keyof typeof appStore];
+      if (app && "lib" in app && "VideoApiAdapter" in app.lib) {
         const makeVideoApiAdapter = app.lib.VideoApiAdapter as VideoApiAdapterFactory;
-      const videoAdapter = makeVideoApiAdapter(cred);
-      videoAdapters.push(videoAdapter);
-    }
-  }
-
-  return videoAdapters;
+        return makeVideoApiAdapter(cred);
+      }
+      return null;
+    })
+  );
+  return adapters.filter((adapter): adapter is VideoApiAdapter => adapter !== null);
 };

🤖 Grapple PR auto-fix • minor • Review this diff before applying


@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MAJOR — Concurrency issues (confidence: 100%)

getVideoAdapters() now uses a sequential for...of loop with await for each credential, serializing all appStore lookups. The previous reduce()-based implementation was synchronous; now N video credentials result in N sequential awaits on import Promises that are already resolved (since all imports fired at module load). While functionally correct, this adds unnecessary sequential microtask overhead when Promise.all would be both simpler and faster.

Evidence:

  • Each await appStore[appName] in the for...of loop is a sequential await on an already-resolved Promise.
  • With N credentials, this creates N sequential microtask checkpoints instead of 1 with Promise.all.
  • The original reduce() was O(N) synchronous; the new for...of is O(N) with N unnecessary sequential awaits.

Agent: performance

@grapple-pr grapple-pr Bot Apr 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — performance agent (Larger fix (22 lines, 1 file) — review recommended)

getVideoAdapters() now uses a sequential for...of loop with await for each credential, serializing all appStore lookups. The previous reduce()-based implementation was synchronous; now N video credentials result in N sequential awaits on import Promises that are already resolved (since all imports fired at module load). While functionally correct, this adds unnecessary sequential microtask overhead when Promise.all would be both simpler and faster.

--- a/packages/core/videoClient.ts
+++ b/packages/core/videoClient.ts
@@ -20,18 +20,15 @@ const translator = short();
 
 // factory
 const getVideoAdapters = async (withCredentials: CredentialPayload[]): Promise<VideoApiAdapter[]> => {
-  const videoAdapters: VideoApiAdapter[] = [];
-
-  for (const cred of withCredentials) {
+  const adapters = await Promise.all(
+    withCredentials.map(async (cred) => {
       const appName = cred.type.split("_").join(""); // Transform `zoom_video` to `zoomvideo`;
       const app = await appStore[appName as keyof typeof appStore];
-
-    if (app && "lib" in app && "VideoApiAdapter" in app.lib) {
-      const makeVideoApiAdapter = app.lib.VideoApiAdapter as VideoApiAdapterFactory;
-      const videoAdapter = makeVideoApiAdapter(cred);
-      videoAdapters.push(videoAdapter);
-    }
-  }
-
-  return videoAdapters;
+      if (app && "lib" in app && "VideoApiAdapter" in app.lib) {
+        const makeVideoApiAdapter = app.lib.VideoApiAdapter as VideoApiAdapterFactory;
+        return makeVideoApiAdapter(cred);
+      }
+      return null;
+    })
+  );
+  return adapters.filter((adapter): adapter is VideoApiAdapter => adapter !== null);
 };

🤖 Grapple PR auto-fix • major • Review this diff before applying

return videoAdapters;
};

const getBusyVideoTimes = (withCredentials: CredentialPayload[]) =>
Promise.all(getVideoAdapters(withCredentials).map((c) => c?.getAvailability())).then((results) =>
const getBusyVideoTimes = async (withCredentials: CredentialPayload[]) =>
Promise.all((await getVideoAdapters(withCredentials)).map((c) => c?.getAvailability())).then((results) =>
results.reduce((acc, availability) => acc.concat(availability), [] as (EventBusyDate | undefined)[])
);

Expand All @@ -45,7 +49,7 @@ const createMeeting = async (credential: CredentialWithAppName, calEvent: Calend
);
}

const videoAdapters = getVideoAdapters([credential]);
const videoAdapters = await getVideoAdapters([credential]);
const [firstVideoAdapter] = videoAdapters;
let createdMeeting;
let returnObject: {
Expand Down Expand Up @@ -104,7 +108,7 @@ const updateMeeting = async (

let success = true;

const [firstVideoAdapter] = getVideoAdapters([credential]);
const [firstVideoAdapter] = await getVideoAdapters([credential]);
const updatedMeeting =
credential && bookingRef
? await firstVideoAdapter?.updateMeeting(bookingRef, calEvent).catch(async (e) => {
Expand Down Expand Up @@ -135,9 +139,9 @@ const updateMeeting = async (
};
};

const deleteMeeting = (credential: CredentialPayload, uid: string): Promise<unknown> => {
const deleteMeeting = async (credential: CredentialPayload, uid: string): Promise<unknown> => {
if (credential) {
const videoAdapter = getVideoAdapters([credential])[0];
const videoAdapter = (await getVideoAdapters([credential]))[0];
// There are certain video apps with no video adapter defined. e.g. riverby,whereby
if (videoAdapter) {
return videoAdapter.deleteMeeting(uid);
Expand All @@ -155,7 +159,7 @@ const createMeetingWithCalVideo = async (calEvent: CalendarEvent) => {
} catch (e) {
return;
}
const [videoAdapter] = getVideoAdapters([
const [videoAdapter] = await getVideoAdapters([
{
id: 0,
appId: "daily-video",
Expand All @@ -178,7 +182,7 @@ const getRecordingsOfCalVideoByRoomName = async (
console.error("Error: Cal video provider is not installed.");
return;
}
const [videoAdapter] = getVideoAdapters([
const [videoAdapter] = await getVideoAdapters([
{
id: 0,
appId: "daily-video",
Expand All @@ -199,7 +203,7 @@ const getDownloadLinkOfCalVideoByRecordingId = async (recordingId: string) => {
console.error("Error: Cal video provider is not installed.");
return;
}
const [videoAdapter] = getVideoAdapters([
const [videoAdapter] = await getVideoAdapters([
{
id: 0,
appId: "daily-video",
Expand Down
23 changes: 12 additions & 11 deletions packages/features/bookings/lib/handleCancelBooking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ async function handler(req: CustomRequest) {
integrationsToDelete.push(deleteMeeting(credential, reference.uid));
}
if (reference.type.includes("_calendar")) {
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
if (calendar) {

@grapple-pr grapple-pr Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Inconsistent Async Pattern (confidence: 79%)

The await getCalendar(credential) calls on lines 243 and 265 are inside a for...of loop iterating over booking references. Each calendar is fetched sequentially. While correct, this is inconsistent with how getCachedResults uses Promise.all for parallel resolution. For cancel booking flows with multiple calendar references, sequential awaiting could add unnecessary latency.

Evidence:

  • Lines 243, 265: const calendar = await getCalendar(credential); inside a for loop
  • CalendarManager.ts line 141 uses Promise.all for the same pattern

Agent: architecture

@grapple-pr grapple-pr Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (32 lines, 1 file) — review recommended)

The await getCalendar(credential) calls on lines 243 and 265 are inside a for...of loop iterating over booking references. Each calendar is fetched sequentially. While correct, this is inconsistent with how getCachedResults uses Promise.all for parallel resolution. For cancel booking flows with multiple calendar references, sequential awaiting could add unnecessary latency.

--- a/packages/features/bookings/lib/handleCancelBooking.ts
+++ b/packages/features/bookings/lib/handleCancelBooking.ts
@@ -240,7 +240,7 @@ async function handler(req: CustomRequest) {
               integrationsToDelete.push(deleteMeeting(credential, reference.uid));
             }
             if (reference.type.includes("_calendar")) {
-              const calendar = getCalendar(credential);
+              const calendar = await getCalendar(credential);
               if (calendar) {
                 integrationsToDelete.push(
                   calendar?.deleteEvent(reference.uid, evt, reference.externalCalendarId)
@@ -262,7 +262,7 @@ async function handler(req: CustomRequest) {
               );
             }
             if (reference.type.includes("_calendar")) {
-              const calendar = getCalendar(credential);
+              const calendar = await getCalendar(credential);
               if (calendar) {
                 integrationsToDelete.push(
                   calendar?.updateEvent(reference.uid, updatedEvt, reference.externalCalendarId)
@@ -449,7 +449,7 @@ async function handler(req: CustomRequest) {
         (credential) => credential.id === credentialId
       );
       if (calendarCredential) {
-        const calendar = getCalendar(calendarCredential);
+        const calendar = await getCalendar(calendarCredential);
         if (
           bookingToDelete.eventType?.recurringEvent &&
           bookingToDelete.recurringEventId &&
@@ -458,10 +458,10 @@ async function handler(req: CustomRequest) {
-          bookingToDelete.user.credentials
-            .filter((credential) => credential.type.endsWith("_calendar"))
-            .forEach(async (credential) => {
-              const calendar = getCalendar(credential);
+          const calendarCredentialsForRecurring = bookingToDelete.user.credentials.filter((credential) =>
+            credential.type.endsWith("_calendar")
+          );
+          for (const credential of calendarCredentialsForRecurring) {
+              const calendar = await getCalendar(credential);
               for (const updBooking of updatedBookings) {
                 const bookingRef = updBooking.references.find((ref) => ref.type.includes("_calendar"));
                 if (bookingRef) {
@@ -471,7 +471,7 @@ async function handler(req: CustomRequest) {
                 }
               }
-            });
+          }
       }
     } else {
       // For bookings made before the refactor we go through the old behaviour of running through each calendar credential
-      bookingToDelete.user.credentials
-        .filter((credential) => credential.type.endsWith("_calendar"))
-        .forEach((credential) => {
-          const calendar = getCalendar(credential);
-          apiDeletes.push(calendar?.deleteEvent(uid, evt, externalCalendarId) as Promise<unknown>);
-        });
+      const calendarCredentials = bookingToDelete.user.credentials.filter((credential) =>
+        credential.type.endsWith("_calendar")
+      );
+      for (const credential of calendarCredentials) {
+        const calendar = await getCalendar(credential);
+        apiDeletes.push(calendar?.deleteEvent(uid, evt, externalCalendarId) as Promise<unknown>);
+      }
     }
   }
 
@@ -586,7 +586,7 @@ async function handler(req: CustomRequest) {
 
     // Posible to refactor TODO:
-    const paymentApp = appStore[paymentAppCredential?.app?.dirName as keyof typeof appStore];
+    const paymentApp = await appStore[paymentAppCredential?.app?.dirName as keyof typeof appStore];
     if (!(paymentApp && "lib" in paymentApp && "PaymentService" in paymentApp.lib)) {
       console.warn(`payment App service of type ${paymentApp} is not implemented`);
       return null;

🤖 Grapple PR auto-fix • minor • Review this diff before applying

integrationsToDelete.push(
calendar?.deleteEvent(reference.uid, evt, reference.externalCalendarId)
Expand All @@ -262,7 +262,7 @@ async function handler(req: CustomRequest) {
);
}
if (reference.type.includes("_calendar")) {
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
if (calendar) {
integrationsToDelete.push(
calendar?.updateEvent(reference.uid, updatedEvt, reference.externalCalendarId)
Expand Down Expand Up @@ -449,7 +449,7 @@ async function handler(req: CustomRequest) {
(credential) => credential.id === credentialId
);
if (calendarCredential) {
const calendar = getCalendar(calendarCredential);
const calendar = await getCalendar(calendarCredential);
if (
bookingToDelete.eventType?.recurringEvent &&
bookingToDelete.recurringEventId &&
Expand All @@ -458,7 +458,7 @@ async function handler(req: CustomRequest) {
bookingToDelete.user.credentials
.filter((credential) => credential.type.endsWith("_calendar"))
.forEach(async (credential) => {
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
for (const updBooking of updatedBookings) {
const bookingRef = updBooking.references.find((ref) => ref.type.includes("_calendar"));
if (bookingRef) {
Expand All @@ -474,12 +474,13 @@ async function handler(req: CustomRequest) {
}
} else {
// For bookings made before the refactor we go through the old behaviour of running through each calendar credential
bookingToDelete.user.credentials
.filter((credential) => credential.type.endsWith("_calendar"))
.forEach((credential) => {
const calendar = getCalendar(credential);
apiDeletes.push(calendar?.deleteEvent(uid, evt, externalCalendarId) as Promise<unknown>);
});
const calendarCredentials = bookingToDelete.user.credentials.filter((credential) =>
credential.type.endsWith("_calendar")
);
for (const credential of calendarCredentials) {
const calendar = await getCalendar(credential);
apiDeletes.push(calendar?.deleteEvent(uid, evt, externalCalendarId) as Promise<unknown>);
}
}
}

Expand Down Expand Up @@ -585,7 +586,7 @@ async function handler(req: CustomRequest) {
}

// Posible to refactor TODO:
const paymentApp = appStore[paymentAppCredential?.app?.dirName as keyof typeof appStore];
const paymentApp = await appStore[paymentAppCredential?.app?.dirName as keyof typeof appStore];
if (!(paymentApp && "lib" in paymentApp && "PaymentService" in paymentApp.lib)) {
console.warn(`payment App service of type ${paymentApp} is not implemented`);
return null;
Expand Down
2 changes: 1 addition & 1 deletion packages/features/bookings/lib/handleNewBooking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,7 @@ async function handler(
integrationsToDelete.push(deleteMeeting(credential, reference.uid));
}
if (reference.type.includes("_calendar") && originalBookingEvt) {
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
if (calendar) {
integrationsToDelete.push(
calendar?.deleteEvent(reference.uid, originalBookingEvt, reference.externalCalendarId)
Expand Down
2 changes: 1 addition & 1 deletion packages/lib/payment/deletePayment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const deletePayment = async (
} | null;
}
): Promise<boolean> => {
const paymentApp = appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
const paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];

@grapple-pr grapple-pr Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MINOR — Error Handling Gap (confidence: 93%)

The await appStore[...] call has no try/catch. If the dynamic import fails (e.g., missing module, syntax error in the imported module), an unhandled exception will propagate up. Given this is in a payment deletion flow, a failed import could leave payments in an inconsistent state.

Evidence:

  • Line 16: const paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
  • Dynamic imports can reject if the module is not found or has errors
  • The same pattern in handlePayment.ts (line 26) and handleCancelBooking.ts (line 589) also lacks try/catch around the dynamic import

Agent: architecture

@grapple-pr grapple-pr Bot Apr 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (13 lines, 1 file) — review recommended)

The await appStore[...] call has no try/catch. If the dynamic import fails (e.g., missing module, syntax error in the imported module), an unhandled exception will propagate up. Given this is in a payment deletion flow, a failed import could leave payments in an inconsistent state.

--- a/packages/lib/payment/deletePayment.ts
+++ b/packages/lib/payment/deletePayment.ts
@@ -13,7 +13,15 @@ const deletePayment = async (
     } | null;
   }
 ): Promise<boolean> => {
-  const paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
+  // Dynamic import may reject if the module is missing or has a syntax error;
+  // return false to signal failure without leaving the caller in an inconsistent state.
+  let paymentApp;
+  try {
+    paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
+  } catch (e) {
+    console.warn(
+      `Payment app module for "${paymentAppCredentials?.app?.dirName}" could not be loaded:`,
+      e
+    );
+    return false;
+  }
   if (!(paymentApp && "lib" in paymentApp && "PaymentService" in paymentApp.lib)) {
     console.warn(`payment App service of type ${paymentApp} is not implemented`);
     return false;

🤖 Grapple PR auto-fix • minor • Review this diff before applying

if (!(paymentApp && "lib" in paymentApp && "PaymentService" in paymentApp.lib)) {
console.warn(`payment App service of type ${paymentApp} is not implemented`);
return false;
Expand Down
2 changes: 1 addition & 1 deletion packages/lib/payment/handlePayment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const handlePayment = async (
uid: string;
}
) => {
const paymentApp = appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
const paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
if (!(paymentApp && "lib" in paymentApp && "PaymentService" in paymentApp.lib)) {
console.warn(`payment App service of type ${paymentApp} is not implemented`);
return null;
Expand Down
Loading