feat: multi-channel connect — account chooser, paginated channel list, picker flow - #68
Merged
Merged
Conversation
🤖 CodeAnt AI — Review Status
|
✅ Deploy Preview for moderaty ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Google OAuth flow now supports account selection and multiple YouTube channels. It retrieves paginated channels, parks multi-channel choices in encrypted cookies, and adds an authenticated picker that persists the selected channel. Cron documentation now describes per-channel scan cadence. ChangesMulti-channel YouTube connection
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:XL
Sequence DiagramThis PR adds account selection and paginated channel discovery to Google OAuth. Accounts with multiple channels park the encrypted token and candidates until the user selects one, while single channel accounts connect immediately. sequenceDiagram
participant User
participant Google
participant Backend
participant PendingCookie
participant Database
User->>Google: Start connect with account chooser
Google->>Backend: OAuth callback with authorization code
Backend->>Google: Exchange code and list all channels
Google-->>Backend: Refresh token and channel candidates
alt One valid channel
Backend->>Database: Connect channel and store encrypted token
Backend-->>User: Redirect to dashboard
else Multiple valid channels
Backend->>PendingCookie: Park token and candidates
Backend-->>User: Redirect to channel picker
User->>Backend: Submit selected channel
Backend->>PendingCookie: Read and consume selected state
Backend->>Database: Connect selected channel and store encrypted token
Backend-->>User: Redirect to dashboard
end
Generated by CodeAnt AI |
|
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This PR implements multi-channel connect functionality with proper security controls. The implementation follows good security practices with encrypted httpOnly cookies, state-keyed storage, and bounded entries.
Critical Issue Found
One logic error in the channel picker action handler that must be fixed before merge:
- Type assertion safety: Line 74 in
connect-channel/+page.server.tsuses unsafe type assertion that could prevent proper cleanup of sensitive tokens
Implementation Quality
The core implementation is solid with good error handling, pagination support, and comprehensive test coverage. The refactoring to extract upsertChannelConnection into a shared utility is a good design choice that prevents code duplication.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
|
|
||
| // Consumed on success only: a transient failure leaves the pick | ||
| // retryable while a success cannot be replayed. | ||
| clearPendingChannelPick(cookies, state as string); |
Contributor
There was a problem hiding this comment.
🛑 Logic Error: The type assertion state as string on line 74 is unsafe since state can be null from line 53. If state is null, this will pass "null" (the string) to clearPendingChannelPick, preventing proper cleanup and potentially leaving sensitive refresh tokens in the cookie.
Suggested change
| clearPendingChannelPick(cookies, state as string); | |
| clearPendingChannelPick(cookies, state!); |
Not up to standards ⛔🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | ✅ 78 (≤ 100 complexity) |
| Duplication |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
PR Code Suggestions ✨Latest suggestions up to commit
|
| Category | Suggestion | Severity | Generated at (UTC) |
| Possible bug |
Parking hundreds of channels and a token in one cookie exceeds browser cookie limitsThe pending cookie stores the refresh token plus every channel returned by a listing src/routes/api/auth/google/callback/+server.ts [136] Why it matters? 🤔
(Use Cmd/Ctrl + Click for best experience) Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** src/routes/api/auth/google/callback/+server.ts
**Line:** 136:136
**Comment:**
*Possible Bug: The pending cookie stores the refresh token plus every channel returned by a listing capped at 500 channels. That payload is far larger than the approximately 4 KB browser cookie limit, and encryption adds further overhead, so browsers will reject or truncate the `Set-Cookie` value for sufficiently large accounts and the picker will immediately report an expired or missing selection. Store the grant and candidate list server-side and put only an opaque state key in the cookie, or enforce a payload size limit before parking it.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix | Major | 2026-08-04 16:10
|
| Race condition |
Concurrent submissions can consume the same pending grant more than onceTwo concurrent POST requests can both read the same still-valid pending entry, src/routes/connect-channel/+page.server.ts [68-74] Why it matters? 🤔
(Use Cmd/Ctrl + Click for best experience) Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** src/routes/connect-channel/+page.server.ts
**Line:** 68:74
**Comment:**
*Race Condition: Two concurrent POST requests can both read the same still-valid pending entry, connect different channels, and only then clear the cookie. Because consumption is not an atomic claim or compare-and-delete operation, one OAuth grant can result in multiple channels being connected. Claim and consume the pending state atomically before performing the upsert, or use durable server-side state with one-time consumption.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix | Major | 2026-08-04 16:10
|
| Api mismatch |
Large multi-channel selections exceed browser cookie limits and make the picker flow unusableThe entire candidate list is serialized into a single browser cookie. With up to 500 src/lib/server/channelConnect.ts [93-97] Why it matters? 🤔
(Use Cmd/Ctrl + Click for best experience) Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** src/lib/server/channelConnect.ts
**Line:** 93:97
**Comment:**
*Api Mismatch: The entire candidate list is serialized into a single browser cookie. With up to 500 channels, the encrypted and base64-encoded payload will exceed common 4 KB cookie limits, causing the `Set-Cookie` to be rejected or truncated and making the picker fail immediately after OAuth. Store the pending token and candidates server-side, or limit and validate the serialized payload before writing the cookie.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix | Major | 2026-08-04 16:10
|
PR Summary by QodoMulti-channel connect: account chooser, paginated listing, picker flow
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@DEPLOY.md`:
- Around line 79-82: Align the schedule documentation at DEPLOY.md lines 79-82
and netlify/functions/cron.mjs lines 60-64: explain that */15 * * * * invokes
the cron every 15 minutes, changing per-channel scan cadence from N to 15 × N
minutes and increasing latency as intentional load shedding, or retain the
one-minute schedule when cadence is the priority. Update both sites
consistently; no code change is required.
In `@src/lib/server/channelConnect.ts`:
- Around line 91-98: Update parkPendingChannelPick to bound each pending pick
before serialization: cap payload.channels to a reasonable maximum candidate
count and truncate each channel title to a fixed maximum length, while
preserving the existing state/timestamp, TTL filtering, and MAX_PENDING_PICKS
limits. Apply the bounds before entries are passed to writeEntries so the
encrypted cookie remains within browser size limits.
In `@src/routes/api/auth/google/callback/`+server.ts:
- Around line 41-95: Extract the per-page YouTube request and response parsing
from fetchOwnedChannels into a dedicated helper, preserving its existing
validation, error responses, and nextPageToken behavior. Keep fetchOwnedChannels
focused on pagination, accumulating valid channels, counting skipped malformed
items, and enforcing MAX_CHANNEL_PAGES; update it to consume the helper’s page
result without changing observable behavior.
In `@src/routes/api/auth/google/callback/callback.test.ts`:
- Around line 191-210: Add a test alongside the existing pagination test that
stubs every channel-list response with a nextPageToken, invokes captureCallback,
and verifies exactly 10 requests occur. Spy on console.error during the test and
assert the truncation message mentions the 10-page bound, restoring the spy
afterward.
In `@src/routes/connect-channel/`+page.svelte:
- Around line 29-51: Update the connect-channel form to use SvelteKit
enhancement with a submitting flag, disabling the “Connect selected channel”
button while the action is running. Add the established EmptyState component
import used by other routes and render it when data.channels.length is zero
instead of the empty fieldset/form content. Preserve the existing form.error
alert and populated channel-selection flow.
In `@src/routes/connect-channel/connect-channel.test.ts`:
- Around line 99-116: Add authorization coverage for the load path using
loadWith: verify a signed-out request rejects with status 401 and a member
request rejects with status 403. Keep the existing parked-channel success test,
and ensure these tests exercise the requireOrgRole gate in the page load
implementation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b06d6b88-3de8-4160-b1c2-9d8b4a81b122
📒 Files selected for processing (10)
DEPLOY.mdnetlify/functions/cron.mjssrc/lib/server/channelConnect.tssrc/routes/api/auth/google/+server.tssrc/routes/api/auth/google/callback/+server.tssrc/routes/api/auth/google/callback/callback.test.tssrc/routes/api/auth/google/oauth.test.tssrc/routes/connect-channel/+page.server.tssrc/routes/connect-channel/+page.sveltesrc/routes/connect-channel/connect-channel.test.ts
Comment on lines
+79
to
+82
| one channel (least-recently-run first), so with N connected channels the | ||
| per-channel scan cadence is N minutes at `* * * * *` (e.g. 5 channels ⇒ each | ||
| scanned every 5 minutes). Raise the schedule frequency if N × interval grows | ||
| past an acceptable cadence. A failed run throws and appears as a failed |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Align the cadence guidance across both files.
*/15 * * * * reduces invocations from once per minute to once per 15 minutes. With one channel per invocation, per-channel cadence changes from N minutes to 15 × N minutes. Document the latency trade-off as intentional load shedding, or keep the one-minute schedule when scan cadence is the priority.
DEPLOY.md#L79-L82: replace “raise the schedule frequency” with precise interval guidance and state the15 × Neffect.netlify/functions/cron.mjs#L60-L64: clarify that*/15 * * * *lowers invocation frequency and increases scan latency.
📍 Affects 2 files
DEPLOY.md#L79-L82(this comment)netlify/functions/cron.mjs#L60-L64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DEPLOY.md` around lines 79 - 82, Align the schedule documentation at
DEPLOY.md lines 79-82 and netlify/functions/cron.mjs lines 60-64: explain that
*/15 * * * * invokes the cron every 15 minutes, changing per-channel scan
cadence from N to 15 × N minutes and increasing latency as intentional load
shedding, or retain the one-minute schedule when cadence is the priority. Update
both sites consistently; no code change is required.
Comment on lines
+91
to
+98
| export function parkPendingChannelPick(cookies: Cookies, state: string, payload: PendingChannelPick): void { | ||
| const now = Date.now(); | ||
| const entries = readEntries(cookies).filter( | ||
| (e) => e.state !== state && now - e.ts <= PICK_TTL_MS | ||
| ); | ||
| entries.push({ ...payload, state, ts: now }); | ||
| writeEntries(cookies, entries.slice(-MAX_PENDING_PICKS)); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the parked channel list to keep the cookie under the browser size limit.
payload.channels arrives from fetchOwnedChannels in src/routes/api/auth/google/callback/+server.ts (line 136), which allows up to 10 pages × 50 items = 500 channels with upstream-controlled titles. Up to MAX_PENDING_PICKS = 5 such entries are serialized into one cookie. A single cookie is limited to about 4096 bytes, and encryption plus base64 expands the payload further.
cookies.set does not report failure. The browser silently drops the oversized cookie, so readPendingChannelPick returns null and the picker load throws 400 "this channel selection expired". Large accounts can then never connect a channel.
Cap the parked candidates and the stored title length.
🛠️ Proposed fix to bound the parked payload
const MAX_PENDING_PICKS = 5;
+// Keeps the encrypted cookie inside the ~4096-byte per-cookie browser limit.
+const MAX_PICK_CHANNELS = 25;
+const MAX_PICK_TITLE_LEN = 60; export function parkPendingChannelPick(cookies: Cookies, state: string, payload: PendingChannelPick): void {
const now = Date.now();
const entries = readEntries(cookies).filter(
(e) => e.state !== state && now - e.ts <= PICK_TTL_MS
);
- entries.push({ ...payload, state, ts: now });
+ const channels = payload.channels
+ .slice(0, MAX_PICK_CHANNELS)
+ .map((c) => ({ id: c.id, title: c.title.slice(0, MAX_PICK_TITLE_LEN) }));
+ entries.push({ ...payload, channels, state, ts: now });
writeEntries(cookies, entries.slice(-MAX_PENDING_PICKS));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function parkPendingChannelPick(cookies: Cookies, state: string, payload: PendingChannelPick): void { | |
| const now = Date.now(); | |
| const entries = readEntries(cookies).filter( | |
| (e) => e.state !== state && now - e.ts <= PICK_TTL_MS | |
| ); | |
| entries.push({ ...payload, state, ts: now }); | |
| writeEntries(cookies, entries.slice(-MAX_PENDING_PICKS)); | |
| } | |
| const MAX_PENDING_PICKS = 5; | |
| // Keeps the encrypted cookie inside the ~4096-byte per-cookie browser limit. | |
| const MAX_PICK_CHANNELS = 25; | |
| const MAX_PICK_TITLE_LEN = 60; | |
| export function parkPendingChannelPick(cookies: Cookies, state: string, payload: PendingChannelPick): void { | |
| const now = Date.now(); | |
| const entries = readEntries(cookies).filter( | |
| (e) => e.state !== state && now - e.ts <= PICK_TTL_MS | |
| ); | |
| const channels = payload.channels | |
| .slice(0, MAX_PICK_CHANNELS) | |
| .map((c) => ({ id: c.id, title: c.title.slice(0, MAX_PICK_TITLE_LEN) })); | |
| entries.push({ ...payload, channels, state, ts: now }); | |
| writeEntries(cookies, entries.slice(-MAX_PENDING_PICKS)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/server/channelConnect.ts` around lines 91 - 98, Update
parkPendingChannelPick to bound each pending pick before serialization: cap
payload.channels to a reasonable maximum candidate count and truncate each
channel title to a fixed maximum length, while preserving the existing
state/timestamp, TTL filtering, and MAX_PENDING_PICKS limits. Apply the bounds
before entries are passed to writeEntries so the encrypted cookie remains within
browser size limits.
Comment on lines
+41
to
+95
| async function fetchOwnedChannels(accessToken: string): Promise<ListedChannel[]> { | ||
| const found: ListedChannel[] = []; | ||
| let skipped = 0; | ||
| let pageToken: string | undefined; | ||
| for (let page = 0; page < MAX_CHANNEL_PAGES; page++) { | ||
| const endpoint = new URL('https://www.googleapis.com/youtube/v3/channels'); | ||
| endpoint.searchParams.set('part', 'snippet'); | ||
| endpoint.searchParams.set('mine', 'true'); | ||
| endpoint.searchParams.set('maxResults', '50'); | ||
| if (pageToken) endpoint.searchParams.set('pageToken', pageToken); | ||
|
|
||
| const chRes = await fetchWithRetry(endpoint.toString(), { | ||
| headers: { Authorization: `Bearer ${accessToken}` } | ||
| }); | ||
| const chText = await chRes.text(); | ||
| if (!chRes.ok) { | ||
| console.error(`youtube channels lookup failed: ${chRes.status}`); | ||
| throw error(502, 'YouTube channel lookup failed — please retry'); | ||
| } | ||
| let chData: { items?: unknown; nextPageToken?: unknown }; | ||
| try { | ||
| chData = JSON.parse(chText) as typeof chData; | ||
| } catch { | ||
| console.error(`youtube channels lookup returned invalid JSON: ${chRes.status}`); | ||
| throw error(502, 'invalid response from YouTube — please retry'); | ||
| } | ||
| if (typeof chData !== 'object' || chData === null) { | ||
| console.error(`youtube channels lookup returned a non-object body: ${chRes.status}`); | ||
| throw error(502, 'invalid response from YouTube — please retry'); | ||
| } | ||
|
|
||
| const items = Array.isArray(chData.items) ? chData.items : []; | ||
| for (const item of items as Array<{ id?: unknown; snippet?: { title?: unknown } }>) { | ||
| if (typeof item?.id === 'string' && item.id) { | ||
| found.push({ | ||
| id: item.id, | ||
| title: typeof item.snippet?.title === 'string' ? item.snippet.title : 'Untitled channel' | ||
| }); | ||
| } else { | ||
| skipped++; | ||
| } | ||
| } | ||
|
|
||
| pageToken = | ||
| typeof chData.nextPageToken === 'string' && chData.nextPageToken ? chData.nextPageToken : undefined; | ||
| if (!pageToken) return finish(found, skipped); | ||
| } | ||
| console.error(`youtube channels lookup hit the ${MAX_CHANNEL_PAGES}-page bound — listing truncated`); | ||
| return finish(found, skipped); | ||
| } | ||
|
|
||
| function finish(found: ListedChannel[], skipped: number): ListedChannel[] { | ||
| if (skipped > 0) console.error(`youtube channels lookup skipped ${skipped} malformed item(s)`); | ||
| return found; | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract the per-page fetch and parse to clear the complexity gate.
SonarCloud fails this function at cognitive complexity 27 against a limit of 15. The validation logic is correct; the page loop just carries transport, parsing, and item validation together. Move one page into its own helper and keep fetchOwnedChannels as the paging loop.
♻️ Proposed extraction
+type ChannelPage = { items: unknown[]; nextPageToken: string | undefined };
+
+async function fetchChannelPage(accessToken: string, pageToken?: string): Promise<ChannelPage> {
+ const endpoint = new URL('https://www.googleapis.com/youtube/v3/channels');
+ endpoint.searchParams.set('part', 'snippet');
+ endpoint.searchParams.set('mine', 'true');
+ endpoint.searchParams.set('maxResults', '50');
+ if (pageToken) endpoint.searchParams.set('pageToken', pageToken);
+
+ const chRes = await fetchWithRetry(endpoint.toString(), {
+ headers: { Authorization: `Bearer ${accessToken}` }
+ });
+ const chText = await chRes.text();
+ if (!chRes.ok) {
+ console.error(`youtube channels lookup failed: ${chRes.status}`);
+ throw error(502, 'YouTube channel lookup failed — please retry');
+ }
+ let chData: unknown;
+ try {
+ chData = JSON.parse(chText);
+ } catch {
+ console.error(`youtube channels lookup returned invalid JSON: ${chRes.status}`);
+ throw error(502, 'invalid response from YouTube — please retry');
+ }
+ if (typeof chData !== 'object' || chData === null) {
+ console.error(`youtube channels lookup returned a non-object body: ${chRes.status}`);
+ throw error(502, 'invalid response from YouTube — please retry');
+ }
+ const body = chData as { items?: unknown; nextPageToken?: unknown };
+ return {
+ items: Array.isArray(body.items) ? body.items : [],
+ nextPageToken:
+ typeof body.nextPageToken === 'string' && body.nextPageToken ? body.nextPageToken : undefined
+ };
+}fetchOwnedChannels then keeps only the loop, the per-item validation, and the page bound.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 41-41: Refactor this function to reduce its Cognitive Complexity from 27 to the 15 allowed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/api/auth/google/callback/`+server.ts around lines 41 - 95, Extract
the per-page YouTube request and response parsing from fetchOwnedChannels into a
dedicated helper, preserving its existing validation, error responses, and
nextPageToken behavior. Keep fetchOwnedChannels focused on pagination,
accumulating valid channels, counting skipped malformed items, and enforcing
MAX_CHANNEL_PAGES; update it to consume the helper’s page result without
changing observable behavior.
Source: Linters/SAST tools
Comment on lines
+191
to
+210
| test('the channel listing paginates and every valid channel reaches the picker', async () => { | ||
| const seenPageTokens: (string | null)[] = []; | ||
| stubTokenAndChannels((url) => { | ||
| seenPageTokens.push(url.searchParams.get('pageToken')); | ||
| if (!url.searchParams.get('pageToken')) { | ||
| return new Response( | ||
| JSON.stringify({ items: [{ id: 'UC1', snippet: { title: 'One' } }], nextPageToken: 'p2' }), | ||
| { status: 200 } | ||
| ); | ||
| } | ||
| return new Response(JSON.stringify({ items: [{ id: 'UC2', snippet: { title: 'Two' } }] }), { status: 200 }); | ||
| }); | ||
| const cookies = makeCookiesWithState('s'); | ||
|
|
||
| const { thrown } = await captureCallback(OWNER, cookies); | ||
|
|
||
| expect(seenPageTokens).toEqual([null, 'p2']); | ||
| expect(thrown).toMatchObject({ status: 302, location: '/connect-channel?state=s' }); | ||
| expect(readPendingChannelPick(cookies as never, 's')?.channels).toHaveLength(2); | ||
| }); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add a test for the page bound.
The pagination test covers two pages. The MAX_CHANNEL_PAGES truncation branch in src/routes/api/auth/google/callback/+server.ts (line 88) has no coverage. A stub that always returns a nextPageToken proves the loop stops at 10 requests and logs the truncation, which protects the serverless time budget.
💚 Proposed test
test('the channel listing stops at the page bound and logs the truncation', async () => {
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
let requests = 0;
stubTokenAndChannels(() => {
requests++;
return new Response(
JSON.stringify({ items: [{ id: `UC${requests}`, snippet: { title: `C${requests}` } }], nextPageToken: 'more' }),
{ status: 200 }
);
});
await captureCallback();
expect(requests).toBe(10);
expect(errSpy.mock.calls.flat().join(' ')).toMatch(/10-page bound/);
errSpy.mockRestore();
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/api/auth/google/callback/callback.test.ts` around lines 191 - 210,
Add a test alongside the existing pagination test that stubs every channel-list
response with a nextPageToken, invokes captureCallback, and verifies exactly 10
requests occur. Spy on console.error during the test and assert the truncation
message mentions the 10-page bound, restoring the spy afterward.
Comment on lines
+29
to
+51
| <main class="pick-main"> | ||
| <div class="card pick-card"> | ||
| <h1>Choose a channel</h1> | ||
| <p class="muted">This Google account owns several YouTube channels. Pick the one Moderaty should moderate.</p> | ||
|
|
||
| {#if form?.error} | ||
| <p class="error-box" role="alert">{form.error}</p> | ||
| {/if} | ||
|
|
||
| <form method="POST"> | ||
| <fieldset> | ||
| <legend>Your channels</legend> | ||
| {#each data.channels as channel (channel.id)} | ||
| <label class="check"> | ||
| <input type="radio" name="channel" value={channel.id} required /> | ||
| <span>{channel.title}</span> | ||
| </label> | ||
| {/each} | ||
| </fieldset> | ||
| <button class="btn" type="submit">Connect selected channel</button> | ||
| </form> | ||
| </div> | ||
| </main> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the loading and empty states required by I12.
This page has the error state (line 35) and the populated state (lines 41-46). It has no submitting state and no empty state. A slow upsertChannelConnection leaves the submit button with no feedback, and a parked list with no candidates renders an empty fieldset with a submit button.
Add use:enhance with a submitting flag, disable the button while the action runs, and render EmptyState when data.channels.length === 0.
🎛️ Proposed states
<script lang="ts">
+ import { enhance } from '$app/forms';
+ import EmptyState from '$lib/components/EmptyState.svelte';
+
let { data, form } = $props();
+ let submitting = $state(false);
</script>- <form method="POST">
+ {`#if` data.channels.length === 0}
+ <EmptyState message="No channels are available to connect. Reconnect the channel from the dashboard." />
+ {:else}
+ <form
+ method="POST"
+ use:enhance={() => {
+ submitting = true;
+ return async ({ update }) => {
+ await update();
+ submitting = false;
+ };
+ }}
+ >
<fieldset>
<legend>Your channels</legend>
{`#each` data.channels as channel (channel.id)}
<label class="check">
<input type="radio" name="channel" value={channel.id} required />
<span>{channel.title}</span>
</label>
{/each}
</fieldset>
- <button class="btn" type="submit">Connect selected channel</button>
+ <button class="btn" type="submit" disabled={submitting}>
+ {submitting ? 'Connecting channel…' : 'Connect selected channel'}
+ </button>
</form>
+ {/if}Confirm the EmptyState import path used by the other routes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/connect-channel/`+page.svelte around lines 29 - 51, Update the
connect-channel form to use SvelteKit enhancement with a submitting flag,
disabling the “Connect selected channel” button while the action is running. Add
the established EmptyState component import used by other routes and render it
when data.channels.length is zero instead of the empty fieldset/form content.
Preserve the existing form.error alert and populated channel-selection flow.
Source: Coding guidelines
Comment on lines
+99
to
+116
| test('load returns the parked channels without ever exposing the refresh token', async () => { | ||
| const data = (await loadWith(cookiesWithPick())) as { channels: unknown }; | ||
|
|
||
| expect(data.channels).toEqual(PICK.channels); | ||
| expect(JSON.stringify(data)).not.toContain('refresh-token'); | ||
| }); | ||
|
|
||
| test('a signed-out picker POST is rejected before any write', async () => { | ||
| const res = await captureAction(cookiesWithPick(), 'UC1', 's', null); | ||
| expect(res).toMatchObject({ status: 401 }); | ||
| expect(await testDb().db.select().from(channels).all()).toHaveLength(0); | ||
| }); | ||
|
|
||
| test('a member cannot complete the picker — 403 before any write', async () => { | ||
| const res = await captureAction(cookiesWithPick(), 'UC1', 's', { ...OWNER, orgRole: 'member' }); | ||
| expect(res).toMatchObject({ status: 403 }); | ||
| expect(await testDb().db.select().from(channels).all()).toHaveLength(0); | ||
| }); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Cover the load authorization gate.
The action gate has tests for 401 and 403 (lines 106-116). The load gate at src/routes/connect-channel/+page.server.ts lines 34-35 has none, so a removed requireOrgRole there would still pass the suite while exposing the parked channel list to a member.
💚 Proposed tests
test('load rejects a signed-out request with 401', async () => {
await expect(loadWith(cookiesWithPick(), 's', null)).rejects.toMatchObject({ status: 401 });
});
test('load rejects a member with 403', async () => {
await expect(
loadWith(cookiesWithPick(), 's', { ...OWNER, orgRole: 'member' })
).rejects.toMatchObject({ status: 403 });
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/connect-channel/connect-channel.test.ts` around lines 99 - 116,
Add authorization coverage for the load path using loadWith: verify a signed-out
request rejects with status 401 and a member request rejects with status 403.
Keep the existing parked-channel success test, and ensure these tests exercise
the requireOrgRole gate in the page load implementation.
Code Review by Qodo
Context used✅ Compliance rules (platform):
78 rules 1. Oversized pending-pick cookie
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+136
to
+137
| parkPendingChannelPick(cookies, state, { refreshToken: tokens.refreshToken, channels: owned }); | ||
| throw redirect(302, `/connect-channel?state=${encodeURIComponent(state)}`); |
There was a problem hiding this comment.
1. Picker redirect url concatenation 📘 Rule violation ≡ Correctness
The new redirect URL is built with a template literal and manual query-string composition instead of using the URL constructor and searchParams. This violates the project rule and can lead to subtle encoding/formatting bugs as URL complexity grows.
Agent Prompt
## Issue description
`/connect-channel` redirect URL is constructed via template literal and manual query parameter concatenation.
## Issue Context
Compliance requires building URLs via `new URL()` + `searchParams` instead of string concatenation.
## Fix Focus Areas
- src/routes/api/auth/google/callback/+server.ts[132-138]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+76
to
+82
| cookies.set(CHANNEL_PICK_COOKIE, encrypt(JSON.stringify(entries)), { | ||
| path: '/', | ||
| httpOnly: true, | ||
| sameSite: 'lax', | ||
| secure: cookieSecure(), | ||
| maxAge: PICK_TTL_MS / 1000 | ||
| }); |
There was a problem hiding this comment.
2. Oversized pending-pick cookie 🐞 Bug ☼ Reliability
The multi-channel flow serializes the entire owned-channel list into a single encrypted cookie, but pagination can produce up to 500 channels and easily exceed common cookie/header size limits, causing the cookie write/send to fail and the picker to read as missing (400). This can make multi-channel connect impossible for large/managed accounts.
Agent Prompt
### Issue description
`parkPendingChannelPick` persists `{refreshToken, channels[]}` in a single encrypted cookie. With pagination enabled (`MAX_CHANNEL_PAGES * maxResults`), this payload can become tens of KB after JSON + AES-GCM overhead + base64, exceeding typical per-cookie / header-size limits and breaking the picker flow.
### Issue Context
The consent flow cookie pattern (`legal.ts`) stores a tiny payload; the new picker stores a potentially huge list.
### Fix Focus Areas
- src/lib/server/channelConnect.ts[47-115]
- src/routes/api/auth/google/callback/+server.ts[123-138]
- src/routes/connect-channel/+page.server.ts[31-70]
### Suggested fix approach
- Change the parked cookie payload to store **only** the refresh token (and ts/state), not the full `channels[]` list.
- In `/connect-channel` **load** (server-side), use the parked refresh token to fetch the owned channel list on demand (same API as the callback). Return only `{id,title}` to the browser.
- In the picker **action**, validate the chosen channel by re-fetching (or by checking membership via a targeted API call) using the parked refresh token, then upsert.
- Keep the existing TTL/entry bound; consider adding a maximum channel count returned to the UI if needed for safety.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+52
to
+55
| const chRes = await fetchWithRetry(endpoint.toString(), { | ||
| headers: { Authorization: `Bearer ${accessToken}` } | ||
| }); | ||
| const chText = await chRes.text(); |
There was a problem hiding this comment.
3. No overall list deadline 🐞 Bug ☼ Reliability
fetchOwnedChannels can perform up to 10 sequential fetchWithRetry calls without an overall deadline, so under throttling/retries the OAuth callback duration can grow unpredictably and exceed serverless/runtime time budgets. This is new risk introduced by replacing the single-page lookup with a multi-page walk.
Agent Prompt
### Issue description
The new pagination loop does not provide an aggregate deadline to `fetchWithRetry`. Since `fetchWithRetry` has its own per-attempt timeout and retry behavior, the total time across up to 10 pages can become large under slow/failed requests.
### Issue Context
`fetchWithRetry` supports an optional `deadline` parameter, and other server-side API calls (e.g., YouTube helpers) thread deadlines through.
### Fix Focus Areas
- src/routes/api/auth/google/callback/+server.ts[41-90]
- src/lib/server/http.ts[19-22]
- src/lib/server/http.ts[117-132]
### Suggested fix approach
- Compute a request-scoped deadline in the callback (e.g., `const deadline = Date.now() + 18_000`) based on your platform’s execution budget.
- Thread that deadline into `fetchOwnedChannels(accessToken, deadline)` and pass it into each `fetchWithRetry(..., ..., deadline)` call.
- Consider early-aborting with a clear 502/504 message when the deadline is exceeded so users can retry cleanly.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
User description
Closes #65. Backend half of multi-channel connect (picker UI polish is the frontend counterpart, #66 — this ships a minimal functional page so the flow works end-to-end).
Behavior
/api/auth/googlenow sendsprompt=consent select_account, so connecting a channel under a different Google account no longer requires the browser to already be signed into it.items[0]with a fullchannels.list?mine=truewalk —pageTokenfollowed to a 10-page (500-channel) bound. I1/I2 hold: malformed items are skipped and counted loudly; a malformed response throws 502.moderaty_channel_pick_pending, 10-minute TTL, bounded at 5 entries — themoderaty_consent_pendingpattern) and the user is redirected to/connect-channel?state=.... The refresh token is never persisted until a channel is chosen./connect-channel: load returns only the{id, title}list (the token never reaches the browser); the action re-validates the session + admin role, rejects any channel id not in the parked list (400), runs the same conditional upsert (cross-team → 409, row untouched), consumes the parked state, and redirects to/dashboard.src/lib/server/channelConnect.ts(upsertChannelConnection) — one ownership guard shared by both the callback and the picker, no copy-paste.netlify/functions/cron.mjsand DEPLOY.md §4 now state the per-channel cadence math (N channels ⇒ N minutes per channel at* * * * *).Tests (written failing-first)
pageToken; both pages' channels reach the picker.Verification
npm run test411/411 ·npm run check0 errors ·npm run buildclean.CodeAnt-AI Description
Let users choose which YouTube channel to connect from a multi-channel Google account
What Changed
Impact
✅ Choose the intended channel from multi-channel Google accounts✅ Fewer accidental channel connections✅ Clearer failures for expired or invalid selections💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.