feat(zoom): discover sessions by host allowlist and Zoom Group - #14517
Open
Subash-Mohan wants to merge 2 commits into
Open
feat(zoom): discover sessions by host allowlist and Zoom Group#14517Subash-Mohan wants to merge 2 commits into
Subash-Mohan wants to merge 2 commits into
Conversation
Both mechanisms walk GET /users/{userId}/recordings, which declares no age
limit, so they reach history older than the meeting-ID path's 15-month cap.
Entries carry the occurrence uuid, topic and start time, so processing needs
no extra call.
Host emails resolve through GET /users rather than trusting the undocumented
convention that {userId} accepts an email; an email matching nobody is
reported instead of quietly indexing nothing. Pages hold 30 recordings because
Zoom expires a next_page_token after 15 minutes and every occurrence on a page
is processed before the next is requested.
A recording's type code decides the session type. Anything outside Zoom's
closed enum is skipped rather than assumed to be a meeting, since the document
id freezes that choice and the access-list endpoint will be picked from it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EBNe54dXPELTXXGdk1uYR6
Contributor
Greptile SummaryThis PR adds Zoom recording discovery through host-email allowlists and Zoom Groups, unions these sources with ID allowlists, and classifies recording entries as meetings, webinars, or unsupported uploads.
Confidence Score: 3/5The PR is not safe to merge until Zoom pagination and host traversal can resume without expired tokens or shifted host positions. Interrupted attempts can restore an expired Zoom page token, and re-resolving a changed host set can reinterpret a positional cursor. Either path can omit recordings from the completed crawl. Files Needing Attention: backend/onyx/connectors/zoom/recordings/discovery.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Resolve configured hosts] --> B[Sort hosts by user ID]
B --> C[Select host by host_index]
C --> D[List recording page]
D --> E[Process recording entries]
E --> F{More recording pages?}
F -->|Yes| G[Checkpoint host_index and Zoom page token]
F -->|No| H[Checkpoint next host_index]
G --> I[Interrupted attempt]
H --> I
I --> J[Fresh connector resolves hosts again]
J --> K[Restore positional index and expiring token]
K --> L[Token may be expired or host index may have shifted]
Prompt To Fix All With AI### Issue 1
backend/onyx/connectors/zoom/recordings/discovery.py:388-395
**Checkpoint Stores Expiring Token**
An interrupted crawl saves Zoom's `next_page_token`, even though Zoom expires it after 15 minutes. A later attempt can send the expired token. A non-retryable response then advances past the host's remaining recordings, while a retryable response can keep failing from the same checkpoint. This can leave recordings unindexed or prevent the crawl from completing.
### Issue 2
backend/onyx/connectors/zoom/recordings/discovery.py:330-353
**Host Cursor Can Shift**
The checkpoint stores only a host's position in a newly resolved and sorted member list. If group membership or active-user resolution changes before an interrupted attempt resumes, an insertion or removal can make that position refer to another host. The crawl can then repeat one host and skip another, leaving the skipped host's recordings unindexed.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(zoom): discover sessions by host al..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+388
to
+395
| if next_page_token: | ||
| return DiscoveryStepResult( | ||
| work=work, | ||
| failures=failures, | ||
| next_cursor={ | ||
| "host_index": position.host_index, | ||
| "page_token": next_page_token, | ||
| }, |
Contributor
There was a problem hiding this comment.
An interrupted crawl saves Zoom's next_page_token, even though Zoom expires it after 15 minutes. A later attempt can send the expired token. A non-retryable response then advances past the host's remaining recordings, while a retryable response can keep failing from the same checkpoint. This can leave recordings unindexed or prevent the crawl from completing.
Knowledge Base Used: Connector ingestion and indexing
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/onyx/connectors/zoom/recordings/discovery.py
Line: 388-395
Comment:
**Checkpoint Stores Expiring Token**
An interrupted crawl saves Zoom's `next_page_token`, even though Zoom expires it after 15 minutes. A later attempt can send the expired token. A non-retryable response then advances past the host's remaining recordings, while a retryable response can keep failing from the same checkpoint. This can leave recordings unindexed or prevent the crawl from completing.
**Knowledge Base Used:** [Connector ingestion and indexing](https://app.greptile.com/onyx/-/custom-context/knowledge-base/onyx-dot-app/onyx/-/docs/connector-ingestion-and-indexing.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+330
to
+353
| # Zoom promises no order. A resumed run resolves again, and the cursor | ||
| # is an index into this list, so a different order would step past a | ||
| # host that was never crawled. | ||
| hosts.sort(key=lambda host: host.user_id) | ||
| self._resolved = hosts | ||
| return hosts, failures | ||
|
|
||
| def discover_step( | ||
| self, | ||
| client: ZoomClient, | ||
| start: SecondsSinceUnixEpoch, | ||
| end: SecondsSinceUnixEpoch, | ||
| cursor: dict[str, Any] | None, | ||
| ) -> DiscoveryStepResult: | ||
| position = ( | ||
| _UserRecordingsCursor.model_validate(cursor) | ||
| if cursor | ||
| else _UserRecordingsCursor() | ||
| ) | ||
| hosts, failures = self._hosts(client, start, end) | ||
| if position.host_index >= len(hosts): | ||
| return DiscoveryStepResult(failures=failures, done=True) | ||
|
|
||
| host = hosts[position.host_index] |
Contributor
There was a problem hiding this comment.
The checkpoint stores only a host's position in a newly resolved and sorted member list. If group membership or active-user resolution changes before an interrupted attempt resumes, an insertion or removal can make that position refer to another host. The crawl can then repeat one host and skip another, leaving the skipped host's recordings unindexed.
Knowledge Base Used: Connector ingestion and indexing
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/onyx/connectors/zoom/recordings/discovery.py
Line: 330-353
Comment:
**Host Cursor Can Shift**
The checkpoint stores only a host's position in a newly resolved and sorted member list. If group membership or active-user resolution changes before an interrupted attempt resumes, an insertion or removal can make that position refer to another host. The crawl can then repeat one host and skip another, leaving the skipped host's recordings unindexed.
**Knowledge Base Used:** [Connector ingestion and indexing](https://app.greptile.com/onyx/-/custom-context/knowledge-base/onyx-dot-app/onyx/-/docs/connector-ingestion-and-indexing.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
|
Full-stack Preview (frontend + backend)
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 11 files
Confidence score: 2/5
backend/onyx/connectors/zoom/client.py: Full-history loads can send a 1970-to-present recording window that Zoom rejects, preventing historical recording discovery; split the request into allowed one-month windows before calling Zoom.backend/onyx/connectors/zoom/recordings/discovery.py: Checkpoint retries can applyhost_indexto a changed host list and skip hosts after membership or active-status changes; persist a stable user ID in the cursor and resume from that host.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/onyx/connectors/zoom/recordings/discovery.py">
<violation number="1" location="backend/onyx/connectors/zoom/recordings/discovery.py:353">
P2: When group membership or active host status changes before a checkpoint retry, `host_index` is applied to a different host list and can skip unprocessed hosts. Persist a stable user ID in the cursor and resume by that ID instead of relying on the list position.</violation>
</file>
<file name="backend/onyx/connectors/zoom/client.py">
<violation number="1" location="backend/onyx/connectors/zoom/client.py:358">
P1: On a full-history load, `_poll_window_dates(0, now)` produces a range from 1970 to today, but Zoom rejects recording requests spanning more than one month. Split the history into allowed one-month windows before calling this endpoint, otherwise host and Group discovery fails before returning any recordings.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| doesn't exist, so swallowing it would turn a mistyped host email into an | ||
| empty index with nothing to explain it.""" | ||
| params: dict[str, Any] = { | ||
| "from": from_date.isoformat(), |
Contributor
There was a problem hiding this comment.
P1: On a full-history load, _poll_window_dates(0, now) produces a range from 1970 to today, but Zoom rejects recording requests spanning more than one month. Split the history into allowed one-month windows before calling this endpoint, otherwise host and Group discovery fails before returning any recordings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/onyx/connectors/zoom/client.py, line 358:
<comment>On a full-history load, `_poll_window_dates(0, now)` produces a range from 1970 to today, but Zoom rejects recording requests spanning more than one month. Split the history into allowed one-month windows before calling this endpoint, otherwise host and Group discovery fails before returning any recordings.</comment>
<file context>
@@ -292,6 +309,69 @@ def list_past_webinar_occurrences(
+ doesn't exist, so swallowing it would turn a mistyped host email into an
+ empty index with nothing to explain it."""
+ params: dict[str, Any] = {
+ "from": from_date.isoformat(),
+ "to": to_date.isoformat(),
+ "page_size": page_size,
</file context>
| if position.host_index >= len(hosts): | ||
| return DiscoveryStepResult(failures=failures, done=True) | ||
|
|
||
| host = hosts[position.host_index] |
Contributor
There was a problem hiding this comment.
P2: When group membership or active host status changes before a checkpoint retry, host_index is applied to a different host list and can skip unprocessed hosts. Persist a stable user ID in the cursor and resume by that ID instead of relying on the list position.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/onyx/connectors/zoom/recordings/discovery.py, line 353:
<comment>When group membership or active host status changes before a checkpoint retry, `host_index` is applied to a different host list and can skip unprocessed hosts. Persist a stable user ID in the cursor and resume by that ID instead of relying on the list position.</comment>
<file context>
@@ -188,10 +229,273 @@ def discover_step(
+ if position.host_index >= len(hosts):
+ return DiscoveryStepResult(failures=failures, done=True)
+
+ host = hosts[position.host_index]
+ from_date, to_date = _poll_window_dates(start, end)
+
</file context>
Zoom expires a next_page_token 15 minutes after issuing it, so a crawl interrupted mid-host resumes holding a dead one. Reporting that abandoned every recording after that page for good, not just for this run: an entity failure ends the attempt COMPLETED_WITH_ERRORS, which Onyx counts as a success, so the next run moves its poll window on and never returns. Discovery now restarts that host from its first page instead. The duplicate fetches are absorbed by the upsert, and the restart is tried once in a row, so a page that reliably outlives the expiry reports and moves on rather than looping. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBNe54dXPELTXXGdk1uYR6
Contributor
There was a problem hiding this comment.
1 existing issue remains and 1 new issue found across 2 files (changes from recent commits).
Confidence score: 2/5
- In
backend/onyx/connectors/zoom/recordings/discovery.py, continuation-page errors other than expired-token failures can restart the host; because the following cursor omitsrestarted, a persistent page error may repeatedly restart or fail to make progress. Restrict restart handling to token expiry and preserve the restart state across the next cursor. - In
backend/onyx/connectors/zoom/recordings/discovery.py, resuming from onlyposition.host_indexcan target the wrong host when group members are re-resolved and reordered, causing a resumed crawl to skip a host. Persist the host identity or a stable host snapshot in the restart cursor.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/onyx/connectors/zoom/recordings/discovery.py">
<violation number="1" location="backend/onyx/connectors/zoom/recordings/discovery.py:387">
P1: When any non-run-fatal error occurs on a continuation page, this branch restarts the host, not just expired-token failures. After the restarted first page succeeds, the next cursor omits `restarted`. A persistent page-2 error therefore restarts forever without reporting a failure or advancing. Restrict this retry to a verified expired-token response or carry a single retry budget through the restarted walk.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
| # so the next run moves its poll window on and never comes back. | ||
| # Walking the host again from its first page costs duplicates the | ||
| # upsert absorbs, and is only tried once in a row. | ||
| if position.page_token and not position.restarted: |
Contributor
There was a problem hiding this comment.
P1: When any non-run-fatal error occurs on a continuation page, this branch restarts the host, not just expired-token failures. After the restarted first page succeeds, the next cursor omits restarted. A persistent page-2 error therefore restarts forever without reporting a failure or advancing. Restrict this retry to a verified expired-token response or carry a single retry budget through the restarted walk.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/onyx/connectors/zoom/recordings/discovery.py, line 387:
<comment>When any non-run-fatal error occurs on a continuation page, this branch restarts the host, not just expired-token failures. After the restarted first page succeeds, the next cursor omits `restarted`. A persistent page-2 error therefore restarts forever without reporting a failure or advancing. Restrict this retry to a verified expired-token response or carry a single retry budget through the restarted walk.</comment>
<file context>
@@ -374,6 +378,27 @@ def discover_step(
+ # so the next run moves its poll window on and never comes back.
+ # Walking the host again from its first page costs duplicates the
+ # upsert absorbs, and is only tried once in a row.
+ if position.page_token and not position.restarted:
+ logger.warning(
+ "Restarting Zoom recordings for %s from the first page: %s",
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
GET /users/{userId}/recordingsdeclares no age limit, so depth is bounded by the account's own recording auto-delete policy instead.How Has This Been Tested?
Unit tests added (258 in the Zoom suite, 1493 across all connectors, all passing). The daily suite gained host-allowlist and Group cases against the real API, which need two new secrets —
zoom-test-host-emailandzoom-test-group-id— and have not been run against a live account yet.Additional Options
Summary by cubic
Adds two optional Zoom discovery mechanisms, a host-email allowlist and a Zoom Group, that union with the existing ID allowlist and reach sessions older than its 15-month cap. Setup is still rejected when all three are empty.
New Features
zoom-test-host-emailandzoom-test-group-id, which haven't been run against a live account yet.Written for commit 63d17b6. Summary will update on new commits.