Skip to content

Comments

Updating file metadata and download actions for sharepoint#19820

Merged
dannyroosevelt merged 27 commits intomasterfrom
danny/sharepoint-actions-file-picker
Feb 19, 2026
Merged

Updating file metadata and download actions for sharepoint#19820
dannyroosevelt merged 27 commits intomasterfrom
danny/sharepoint-actions-file-picker

Conversation

@dannyroosevelt
Copy link
Collaborator

@dannyroosevelt dannyroosevelt commented Jan 23, 2026

WHY

Summary by CodeRabbit

  • New Features

    • Added "Download Files" and "Retrieve File Metadata" SharePoint actions, a reusable SharePoint file-picker backend, Graph/SharePoint integration (client, Graph requests, REST helpers), webhook subscription management, fileIds selection, and a new real-time Updated File source.
    • Enhanced file picker UI with search, icons, metadata (size, last modified, child count), and pre-populated selections.
  • Bug Fixes

    • Removed legacy Select Files action.
  • Chores

    • Bumped SharePoint package version and related runtime updates; added webhook timing constants.

Prototypes — not ready to ship
@vercel
Copy link

vercel bot commented Jan 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
pipedream-docs-redirect-do-not-edit Ignored Ignored Feb 19, 2026 4:39pm

Request Review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds Microsoft Graph and SharePoint REST integrations, shared file-picker utilities, new SharePoint actions/sources (download-files, retrieve-file-metadata, updated-file-instant), ConfigureFilePicker UI enhancements, constants/utilities, package bumps, and many minor version increments and trailing-newline fixes.

Changes

Cohort / File(s) Summary
SharePoint Core & Utilities
components/sharepoint/common/file-picker-base.mjs, components/sharepoint/common/utils.mjs, components/sharepoint/common/constants.mjs
Adds shared file-picker props/methods (categorizeSelections, fetchFileMetadata, processResults, etc.), parsing/unwrapping and helper utilities (parseFileOrFolder*, tryDecodeBase64, getAccessLevel), and webhook timing constants/exports.
SharePoint App Enhancements
components/sharepoint/sharepoint.app.mjs, components/sharepoint/package.json
Introduces Microsoft Graph client, retryable graphRequest, SharePoint REST helpers, webhook subscription create/update/delete, delta support, new fileIds prop, enriched item metadata (lastModifiedDateTime, childCount), and dependency bumps.
New SharePoint Actions
components/sharepoint/actions/download-files/download-files.mjs, components/sharepoint/actions/retrieve-file-metadata/retrieve-file-metadata.mjs
Adds two new actions to download files and retrieve file metadata; both use file-picker helpers to parse selections, validate input, categorize files/folders, fetch metadata (optionally include download URLs), and format results with per-item error handling.
Removed Action
components/sharepoint/actions/select-files/select-files.mjs
Removes the legacy select-files action (previous multi-path file/folder selection and parallel metadata aggregation).
Action/Source Version Bumps
components/sharepoint/actions/..., components/sharepoint/sources/... (multiple files)
Multiple SharePoint action and source modules had minor version increments; no behavioral changes.
New/Updated SharePoint Source
components/sharepoint/sources/updated-file-instant/updated-file-instant.mjs
New webhook-based updated-file instant source implementing subscription lifecycle (activate/deactivate), delta tracking, renewal, webhook validation, notification filtering, downloadUrl resolution, and event emission.
React ConfigureFilePicker
packages/connect-react/src/components/ConfigureFilePicker/index.tsx, packages/connect-react/package.json, packages/connect-react/CHANGELOG.md
Enhances ConfigureFilePicker with per-page search, pre-selection from initialConfiguredProps, SVG icons, size/lastModified/childCount metadata, multi-select/Select All, UI layout refinements; package version bumped to 2.7.0.
Misc: Trailing Newlines & Minor Files
multiple components/*/*.app.mjs, components/mixpanel_service_account/..., components/microsoft_sharepoint_dev/..., components/dataiku/..., components/logfire/..., components/sharepoint_admin/..., components/voluum/..., components/voluum_session_auth/..., components/ukg_pro/...
Added missing trailing newlines and small metadata-only edits across many component files; no functional changes.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Picker as ConfigureFilePicker
    participant SharePoint as SharePoint App
    participant Graph as Microsoft Graph

    User->>Picker: Open picker, search/select items
    Picker->>SharePoint: resolveWrappedArrayValues / resolveWrappedValue
    SharePoint->>Graph: graphRequest (get drive/item metadata)
    Graph-->>SharePoint: metadata (incl. lastModifiedDateTime, childCount)
    SharePoint-->>Picker: enriched items

    User->>Picker: Trigger "Download Files" action
    Picker->>SharePoint: filePickerMethods.fetchFileMetadata(files, includeDownloadUrl=true)
    SharePoint->>Graph: graphRequest (get download URLs)
    Graph-->>SharePoint: files with downloadUrl
    SharePoint-->>Picker: processResults(response)
    Picker-->>User: return formatted result
Loading
sequenceDiagram
    participant Source as updated-file-instant
    participant Graph as Microsoft Graph
    participant Store as Datastore
    participant User

    Source->>Graph: createSubscription(resource, notificationUrl, clientState)
    Graph-->>Source: subscriptionId + expirationDateTime
    Source->>Store: save subscription metadata + monitoredFileIds + deltaLink

    Webhook->>Source: validation request (validationToken)
    Source-->>Webhook: echo validationToken

    Webhook->>Source: notification (clientState + resource data)
    Source->>Store: validate clientState
    Source->>Graph: getDriveDelta / getDriveItem for changed files
    Graph-->>Source: changed file metadata (ensure downloadUrl)
    Source->>Store: update deltaLink
    Source-->>User: emit events for updated files

    loop periodic
        Source->>Graph: updateSubscription (renew)
        Graph-->>Source: success or error (handle 404/401/403)
    end

    Source->>Graph: deleteSubscription(subscriptionId)
    Source->>Store: clear subscription state
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Suggested labels

ai-assisted

Suggested reviewers

  • jcortes
  • lcaresia
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description only contains a placeholder '## WHY' with no actual content or explanation of the changes. Complete the '## WHY' section with details about the motivation, objectives, and implementation details of these changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main changes: updates to file metadata and download actions for SharePoint across multiple action files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch danny/sharepoint-actions-file-picker

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Comment @coderabbitai help to get the list of available commands and usage tips.

dannyroosevelt and others added 11 commits February 5, 2026 11:20
search, select / deselect all, surfacing additional metadata
Resolved conflicts by:
- Keeping master's version bumps across all action/source files
- Merging both HEAD's new methods (ACLs, groups, delta tracking) and master's methods in sharepoint.app.mjs
- Combining constants from both branches in constants.mjs
- Merging utility functions from both branches in utils.mjs
- Accepting master's pnpm-lock.yaml
- Resolving file rename/modification conflicts

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@dannyroosevelt dannyroosevelt marked this pull request as ready for review February 14, 2026 17:19
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 19

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
components/sharepoint/actions/update-item/update-item.mjs (1)

44-59: 🧹 Nitpick | 🔵 Trivial

Optional: Consider mapping SharePoint column types to appropriate prop types.

The additionalProps method currently types all dynamically generated column properties as "string" (line 53), regardless of the actual SharePoint column type. SharePoint supports various column types (number, boolean, date, choice, lookup, etc.) that may benefit from more specific type annotations for better validation and developer experience.

Note: This is existing code and not directly related to the version bump in this PR.

🤖 Fix all issues with AI agents
In `@components/sharepoint/actions/get-file-permissions/get-file-permissions.mjs`:
- Around line 487-492: The loop currently assigns the same usersArray (from
usersWithAccess) and expansionDebug to every result item, which is misleading;
instead, remove per-item assignment of usersWithAccess and _expansionDebug in
the for..of loop (referencing usersWithAccess, usersArray, results,
expansionDebug) and attach usersWithAccess (as an array) and _expansionDebug to
the top-level response object returned by the function; alternatively, if you
must keep per-item fields, add a clear comment above the assignment stating this
is a global list for all files, but preferred fix is to delete the per-item
assignments and include usersWithAccess: Array.from(usersWithAccess.values())
and _expansionDebug in the final return payload.

In `@components/sharepoint/actions/list-site-members/list-site-members.mjs`:
- Around line 470-547: The loop over sharePointSiteGroups in
list-site-members.mjs (see sharePointSiteGroups, getSharePointSiteGroupMembers,
groupsFromPermissions, usersFromPermissions) can call the API once per group and
cause latency/throttling; modify the code to make expansion optional via a new
flag/prop (e.g., expandSharePointGroups) and add a configurable cap (e.g.,
maxSharePointGroupsToExpand) so you only process the first N groups, and replace
the sequential awaits with a bounded-parallel approach (Promise.all with a
concurrency limiter or a simple worker pool) to batch calls to
getSharePointSiteGroupMembers and still collect results into
groupsFromPermissions and usersFromPermissions while preserving error handling
per-group.
- Around line 344-401: The helper expandSharePointGroup defined inside run()
mutates outer-scope usersFromPermissions and the group object, so extract it
into a standalone function (e.g., exportable async function
expandSharePointGroup) that takes explicit parameters (group, site,
sharePointSiteGroups, $ or context) and returns a result object containing
{usersToAdd: [...], updatedGroup: {...}, success: boolean, memberCount?,
error?}; in run() call this function, merge the returned usersToAdd into
usersFromPermissions and replace or assign group properties from updatedGroup
instead of letting the helper mutate closure variables, and handle errors by
setting group.type/expandError based on the returned error flag.

In `@components/sharepoint/actions/list-users/list-users.mjs`:
- Around line 215-217: The OData filter construction in the ListUsers logic uses
this.searchQuery directly (the code pushing into filters with
startswith(displayName,'...')), which breaks on single quotes and risks OData
injection; fix by sanitizing/escaping single quotes in this.searchQuery before
interpolation (e.g., replace each ' with '' per OData escaping) or introduce a
small helper like escapeODataString and use that when building the filter so
filters.push(`startswith(displayName,'${escapeODataString(this.searchQuery)}')`)
is used instead of interpolating raw input.

In `@components/sharepoint/common/constants.mjs`:
- Around line 6-17: The WEBHOOK_SUBSCRIPTION_EXPIRATION_TIME_MILLISECONDS
constant is set to 30 days which exceeds Microsoft Graph's max (42,300 minutes);
change WEBHOOK_SUBSCRIPTION_EXPIRATION_TIME_MILLISECONDS to use 29 days in
milliseconds instead (29 * 24 * 60 * 60 * 1000) and keep
WEBHOOK_SUBSCRIPTION_RENEWAL_SECONDS computed from that constant
((WEBHOOK_SUBSCRIPTION_EXPIRATION_TIME_MILLISECONDS * 0.95) / 1000) so renewal
stays at 95% of the new safe expiration.

In `@components/sharepoint/common/file-picker-base.mjs`:
- Around line 193-214: The code currently mutates the wrong object and leaves
duplicate URL properties; after building result (in the map that creates result
from file), if includeDownloadUrl is true assign result.downloadUrl =
file["@microsoft.graph.downloadUrl"] and then delete
result["@microsoft.graph.downloadUrl"], otherwise delete
result["@microsoft.graph.downloadUrl"] and ensure result.downloadUrl is not
present; update the logic around the includeDownloadUrl check (referencing
includeDownloadUrl, result, file and the "@microsoft.graph.downloadUrl"
property) so the final returned object only contains downloadUrl when requested
and never retains the original `@microsoft.graph.downloadUrl` key.

In `@components/sharepoint/common/utils.mjs`:
- Around line 80-85: The getAccessLevel utility currently calls
roles.includes(...) which will throw if roles is null/undefined; update
getAccessLevel to defensively handle missing or non-array input by normalizing
roles at the top (e.g., if (!Array.isArray(roles)) roles = [] or roles = roles
|| []), then run the existing includes checks and return "read" by default;
ensure you only change the getAccessLevel function and preserve its existing
return order ("owner", "write", "read").

In `@components/sharepoint/sharepoint.app.mjs`:
- Around line 470-481: The retry bail list currently checks status via "const
status = error.statusCode || error.response?.status" and bails by calling
"bail(error)" for 401, 403, 404; update that check to also include 400 so Bad
Request is treated as a client error that should not be retried (i.e., add 400
to the array passed to the includes(...) call that triggers bail(error)).
- Around line 426-437: The client() method currently creates a new Microsoft
Graph Client on every call which is wasteful; modify client() to memoize the
Client instance on the class (e.g., this._graphClient) and return the cached
instance when present, while keeping the authProvider.getAccessToken
implementation that calls this._getAccessToken() so token refresh continues to
work; additionally add a clear/reset (or recreate) hook to invalidate
this._graphClient when authentication state changes (logout/token revocation) so
the cached client can be recreated with fresh state.
- Around line 597-604: The getSharePointSiteGroupMembers method currently
interpolates groupId directly into the REST path; validate groupId (e.g.,
confirm it's a finite integer or numeric string convertible to an integer using
Number.isInteger or Number.isFinite after coercion) before building the path,
and if invalid throw a clear error or return a rejected Promise; update
getSharePointSiteGroupMembers to perform this check and only interpolate a
sanitized/parsed integer into the path (refer to the
getSharePointSiteGroupMembers function and the path
`/web/sitegroups/getbyid(${groupId})/users`), ensuring no path injection can
occur.

In `@components/sharepoint/sources/updated-file-instant/updated-file-instant.mjs`:
- Around line 263-267: The code directly invokes the lifecycle hook via
this.hooks.activate.call(this), which can cause double initialization; extract
the subscription creation logic into a new helper method (e.g.,
_createSubscription or createSubscription) that contains the current
subscription setup logic, update activate() to call that new method, and replace
the direct this.hooks.activate.call(this) call in the renewal path with await
this._createSubscription() so both paths reuse the same implementation without
invoking lifecycle hooks directly.
- Around line 344-352: The delta response may omit
file["@microsoft.graph.downloadUrl"], so update the loop that emits changedFiles
to handle missing download URLs: inside the for (const file of changedFiles)
loop (where you call this.$emit and this.generateMeta), check if
file["@microsoft.graph.downloadUrl"] is present and if not, fetch fresh metadata
for that file (e.g., call the Graph files API or the existing metadata helper
used elsewhere) to obtain the download URL before emitting; alternatively
explicitly allow undefined by emitting downloadUrl: undefined and document that
behavior—ensure the fix references the same symbols (changedFiles,
file["@microsoft.graph.downloadUrl"], this.$emit, generateMeta) so the emit
always has a reliable value or a documented undefined.
- Around line 286-297: The current loop over body.value rejects the whole batch
on the first clientState mismatch by calling this.http.respond and returning;
instead, filter out invalid notifications and only reject if no notifications
remain. Replace the for-loop that checks notification.clientState with logic
that builds a validNotifications array (e.g., const validNotifications =
body.value.filter(n => n.clientState === clientState)), log or count invalid
ones for diagnostics, and continue processing validNotifications; only call
this.http.respond(401, ...) if validNotifications.length === 0, otherwise
proceed with normal handling of validNotifications.

In `@packages/connect-react/CHANGELOG.md`:
- Around line 9-14: Update the changelog entry for ConfigureFilePicker to remove
the redundant phrase "SVG graphics" and replace it with a tighter term such as
"SVGs" or "SVG icons" so the bullet reads e.g. "Improved default icons for
folders and files with SVG icons"; keep the rest of the ConfigureFilePicker
bullets unchanged.

In `@packages/connect-react/src/components/ConfigureFilePicker/index.tsx`:
- Around line 62-73: The formatFileSize function can index past the sizes array
for huge byte values; update the sizes array to include larger units (e.g.,
"TB", "PB", "EB") and clamp the computed index i to Math.min(i, sizes.length -
1) before using sizes[i] so it never becomes undefined; modify the logic in
formatFileSize to compute i as Math.floor(Math.log(bytes) / Math.log(k)) then
clamp it and use the clamped index when computing the scaled value and formatted
string.
- Around line 1076-1103: The list item currently uses <li role="button"> with
click/keyboard/mouse handlers which weakens semantics; change the markup so the
<li> is static and contains a real <button> for the interactive element, move
onClick/onKeyDown/onMouseEnter/onMouseLeave/tabIndex and the dynamic background
style to that <button>, keep the style spread (styles.item) and selected logic
(theme.colors.primary25, theme.colors.neutral5) applied to the button, and keep
handleItemClick as the click handler; ensure the <li> no longer has
role="button" or tabIndex so accessibility and screen‑reader semantics are
correct.
- Around line 434-463: The selectedItems initialization (state created by
selectedItems / setSelectedItems in ConfigureFilePicker) currently omits
childCount and lastModifiedDateTime when reconstructing FilePickerItem from
initialConfiguredProps[fileOrFolderProp]; update the parsing logic that builds
items.push in the initializer to include childCount (as number | undefined) and
lastModifiedDateTime (as string | undefined) pulled from parsed (e.g., cast
parsed to Record<string, unknown> and read childCount and lastModifiedDateTime),
so restored FilePickerItem objects contain those fields before being stored in
selectedItems and later returned on confirm.
- Around line 46-56: The FolderIcon and FileIcon SVG components are missing
accessible text; update both components (FolderIcon and FileIcon) to include a
<title> element with a unique, descriptive label (e.g., "Folder" and "File") and
ensure the SVGs have aria-labelledby pointing to that title (or alternatively
add an explicit aria-label) so screen readers can announce them; make sure IDs
used for aria-labelledby are unique if multiple icons may appear on the page.
- Around line 650-657: The unconditional console.log calls in
ConfigureFilePicker that print allItems[0].raw and parsed fields can leak
metadata; replace them by gating behind the component's debug/logging mechanism
(use the existing log helper or a debug flag) so the prints only run when debug
is enabled—locate the block in ConfigureFilePicker where allItems is inspected
(references: allItems, allItems[0].raw, size, childCount, lastModifiedDateTime)
and change the console.log invocations to call the log helper (or wrap them in
if (debug) { ... }) so production builds no longer emit these messages.

This commit addresses all legitimate CodeRabbit review comments and fixes
several critical issues identified during code review.

Backend fixes (SharePoint components):
- Fix critical downloadUrl bug in file-picker-base where property was deleted from wrong object
- Fix webhook expiration exceeding Graph API limit (reduced from 30 to 29 days)
- Add OData injection prevention in list-users search query
- Improve clientState validation to filter invalid notifications instead of rejecting batch
- Handle missing downloadUrl in delta API responses by fetching fresh metadata
- Add null safety to getAccessLevel utility function
- Add Graph client caching to prevent duplicate client instances
- Add 400 status to retry bail list for better error handling

Frontend fixes (connect-react ConfigureFilePicker):
- Add accessibility: SVG title elements and semantic button with keyboard support
- Fix formatFileSize overflow by capping max unit index
- Preserve metadata (size, childCount, lastModifiedDateTime) when parsing items
- Gate debug logging to avoid unnecessary processing
- Fix CHANGELOG wording: change "SVG graphics" to "SVGs"

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@components/sharepoint/common/file-picker-base.mjs`:
- Around line 119-147: The constructSharePointViewUrl function currently only
recognizes /sites/... URLs and returns null for /teams/...; update the regexes
used to extract libraryUrlPart and siteUrl (the two matches where /sites/ is
hardcoded) to accept either "sites" or "teams" (e.g., use a group like
(?:sites|teams)) so team-site URLs are handled; keep the rest of the logic the
same (use libraryUrlPart, siteUrl, decodeURIComponent(url.pathname), parentPath,
and build the AllItems.aspx URL as before).

In `@packages/connect-react/src/components/ConfigureFilePicker/index.tsx`:
- Around line 985-1000: The search input in ConfigureFilePicker (the <input>
using value={searchQuery} and onChange={(e) => setSearchQuery(e.target.value)})
lacks an accessible name; add an aria-label (for example aria-label="Search
files") to that input element so screen readers can announce its purpose even
after the placeholder disappears.
- Around line 86-94: The formatDate function currently hardcodes "en-US", which
breaks i18n; update the formatDate function to accept an optional locale
parameter (e.g., locale?: string) and pass that to toLocaleDateString (or
default to undefined or navigator.language to use the user's browser locale),
and update any callers of formatDate in this component to pass through a locale
if available; reference the formatDate function in ConfigureFilePicker and
ensure the default behavior preserves existing output when no locale is
provided.

@dannyroosevelt dannyroosevelt changed the title [draft] users and acls actions for sharepoint Updating file metadata and download actions for sharepoint Feb 18, 2026
Resolved pnpm-lock.yaml conflict by regenerating lockfile
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
components/logfire/logfire.app.mjs (1)

1-11: 🧹 Nitpick | 🔵 Trivial

Remove unrelated file from SharePoint-focused PR.

This Logfire component is unrelated to the PR's stated objectives (SharePoint file metadata and download actions). The only change is adding a trailing newline, which has no functional impact. Including unrelated files creates noise and makes the PR harder to review.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/logfire/logfire.app.mjs` around lines 1 - 11, This file exports a
Logfire app object (export default { type: "app", app: "logfire", methods: {
authKeys() { ... }}}) which is unrelated to the SharePoint metadata/download PR;
remove the file from the change set (undo the addition or exclude it from the
commit/branch) so only SharePoint-related changes remain, ensuring no unrelated
artifacts like the authKeys method or trailing newline are included in this PR.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/dataiku/dataiku.app.mjs`:
- Line 11: Remove the accidental trailing newline addition in the Dataiku
component change: revert the unrelated whitespace edit so the file ends exactly
as before (no extra blank line after the closing "};"), ensuring the only
changes in this PR remain the SharePoint-related edits and the Dataiku file is
restored to its original state.

In `@components/voluum_session_auth/voluum_session_auth.app.mjs`:
- Line 11: A trailing newline was introduced in the voluum_session_auth module
(the file ending with the token "};") that is unrelated to this PR; revert that
cosmetic change by restoring the file to its previous state (remove the
incidental formatting-only edit around the final "};") and, if you want to apply
formatter changes, put them in a separate formatting-only PR so this feature PR
stays focused.

In `@components/voluum/voluum.app.mjs`:
- Line 11: This change only adds a trailing newline (the lone diff shows the
closing token "};") in components/voluum/voluum.app.mjs which appears unrelated
to the SharePoint metadata/download work; either remove this file change from
the PR (revert the whitespace-only edit around the "};") or explicitly
justify/describe why the components/voluum/voluum.app.mjs newline was
intentionally included in the PR description and commit message so reviewers
know it’s deliberate.

---

Outside diff comments:
In `@components/logfire/logfire.app.mjs`:
- Around line 1-11: This file exports a Logfire app object (export default {
type: "app", app: "logfire", methods: { authKeys() { ... }}}) which is unrelated
to the SharePoint metadata/download PR; remove the file from the change set
(undo the addition or exclude it from the commit/branch) so only
SharePoint-related changes remain, ensuring no unrelated artifacts like the
authKeys method or trailing newline are included in this PR.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/sharepoint/sources/updated-file-instant/updated-file-instant.mjs`:
- Around line 282-303: The code silently drops all notifications when
this._getSubscription() returns null because clientState becomes undefined; add
a defensive check right after calling this._getSubscription() that if
subscription is null/undefined you log a warning (include function/class context
and that subscription is missing) and immediately respond via this.http.respond
with an appropriate error status (e.g., 400 or 500) and short body, instead of
proceeding to compute validNotifications; reference _getSubscription(),
clientState, validNotifications and this.http.respond so the fix is placed
before the validNotifications filter.
- Around line 235-242: generateMeta currently uses
Date.parse(file.lastModifiedDateTime) which can be NaN for missing/malformed
values; update generateMeta to validate the parsed value (const ts =
Date.parse(file.lastModifiedDateTime)) and if Number.isNaN(ts) replace it with a
safe fallback (e.g., Date.now() or 0) before building the id and ts fields so
you never produce "fileId-NaN" or a NaN timestamp; keep the id format
`${file.id}-${ts}` and optionally add a debug/processLogger.warn mentioning the
malformed lastModifiedDateTime for easier tracing.

---

Duplicate comments:
In `@components/sharepoint/sources/updated-file-instant/updated-file-instant.mjs`:
- Around line 263-267: The code directly invokes the lifecycle hook
this.hooks.activate.call(this), which risks double initialization; refactor the
subscription creation logic out of the activate hook into a new reusable method
(e.g., createOrRecreateSubscription or ensureSubscription) and call that method
here instead, updating the activate implementation to call the new method as
well so both paths reuse the same logic and avoid calling
this.hooks.activate.call(this) directly.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/connect-react/src/components/ConfigureFilePicker/index.tsx`:
- Line 511: Remove the stray empty line causing the ESLint
no-multiple-empty-lines error in the ConfigureFilePicker component file: open
the ConfigureFilePicker component (the React component or its export, e.g.,
ConfigureFilePicker / export default ConfigureFilePicker) and delete the extra
blank line at the reported location so there are no consecutive empty lines;
ensure surrounding code retains a single separating newline where appropriate.
- Around line 401-404: The no-arg useCallback named log causes TypeScript errors
because callers pass arguments; update the log callback returned by useCallback
in ConfigureFilePicker to accept rest parameters (e.g., (...args: any[]) or
(...args: unknown[])) and forward them to the real logger or console when
debug/logging is enabled; ensure the signature change is applied where log is
declared so all call sites (calls at lines referenced in the review) match the
function type and the implementation conditionally no-ops when debug is false.

---

Duplicate comments:
In `@packages/connect-react/src/components/ConfigureFilePicker/index.tsx`:
- Around line 946-961: The search input in ConfigureFilePicker uses only a
placeholder which is not accessible once typing starts; add an aria-label (e.g.,
aria-label="Search files") to the input element to provide a persistent
accessible name for screen readers—locate the JSX input that binds
value={searchQuery} and onChange={(e) => setSearchQuery(e.target.value)} and add
the aria-label attribute to it.

@dannyroosevelt dannyroosevelt merged commit 16457f2 into master Feb 19, 2026
9 checks passed
@dannyroosevelt dannyroosevelt deleted the danny/sharepoint-actions-file-picker branch February 19, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants