Skip to content

feat: honest last-checked, no history on connect, opt-in analyze-history window - #69

Merged
Bonobo791 merged 2 commits into
mainfrom
fix-last-checked-scan-window
Aug 4, 2026
Merged

Bonobo791 merged 2 commits into
mainfrom
fix-last-checked-scan-window

Conversation

@Bonobo791

@Bonobo791 Bonobo791 commented Aug 4, 2026 •

Copy link
Copy Markdown
Collaborator

User description

Behavior

Bug fix — "last checked": the dashboard labeled the newest comment seen (channels.cursor) as "last checked", so a channel whose newest comment is ~5 years old read "last checked 258 weeks ago". The card now renders channels.lastRunAt (the actual last cron run), which the load now projects. relative-time also gains month/year units so old timestamps stop degrading to giant week counts.

Scan window — how far back the app pulls comments (changed per product decision): previously a newly connected channel had NO time boundary — cron drained the channel's entire comment history, years back, at 300 comments per run. Now:

  • Connect analyzes nothing historical. A new channel row starts with cursor = createdAt, so only comments published after connection are moderated. Reconnects (token handover) keep the existing window.
  • "Analyze history" is explicit and bounded. Each dashboard channel card has an "Analyze history" control with a window select (1 / 3 / 6 / 12 / 24 months). The action moves the scan boundary back that far and resets the drain state; cron then pages through that window in normal bounded runs. Already-seen comments are skipped before scoring (decideNewComments dedupes by id), so only unscanned history costs YouTube/AI calls. Preset windows only — an unbounded window is an unbounded API cost (I10). Tenancy-scoped like every channel mutation (another team's channel → 404).

Existing channels are unaffected — their cursors are already established; no backfill.

Tests (failing first)

  • New connect → cursor = createdAt; reconnect preserves the pre-existing cursor.
  • analyzeHistory: boundary lands ~N×30 days back, drain state (nextPageToken/scanCursor) reset; invalid months (0/2/25/'x') → 400 unchanged; unknown/cross-team channel → 404; signed-out → 401.
  • Dashboard load projects lastRunAt; relative-time month/year buckets incl. singular forms.

Verification

npm run test 420/420 · npm run check 0 errors · npm run build clean.

Note: touches one dashboard svelte form + relative-time.ts (frontend layer) — minimal, functional styling only; visual polish is the frontend owner's call.


CodeAnt-AI Description

Make scan history explicit and show the actual last check time

What Changed

  • Newly connected channels analyze only comments posted after connection instead of scanning their entire history.
  • Users can start a bounded history scan for the last 1, 3, 6, 12, or 24 months from each dashboard channel card.
  • History scans reset safely after an active scan finishes; users receive a clear retry message if a scan is already running.
  • Dashboard cards now show when moderation last ran, with older dates displayed in months or years instead of large week counts.
  • Invalid history ranges, unavailable channels, and signed-out requests are rejected without changing scan progress.

Impact

✅ No unexpected historical moderation on channel connection
✅ Controlled moderation cost for history scans
✅ Accurate last-checked timestamps

💡 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:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

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:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

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.

@cla-bot cla-bot Bot added the cla-signed label Aug 4, 2026
@codeant-ai

codeant-ai Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Incremental review completed bcbaf4d Aug 04, 2026 · 17:29 17:31
✅ Reviewed your PR b163475 Aug 04, 2026 · 17:09 17:11

@netlify

netlify Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for moderaty ready!

Name Link
🔨 Latest commit bcbaf4d
🔍 Latest deploy log https://app.netlify.com/projects/moderaty/deploys/6a72216b3efa9800085c5ab9
😎 Deploy Preview https://deploy-preview-69--moderaty.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 87
Accessibility: 97
Best Practices: 100
SEO: 100
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added dashboard controls to analyze comment history for selectable periods from 1 to 24 months.
    • Historical analysis now resets scanning so selected past comments can be reviewed.
  • Improvements

    • Newly connected channels begin monitoring from their connection time, excluding older comments by default.
    • Dashboard timestamps now accurately show when each channel was last checked.
    • Relative timestamps now support weeks, months, and years with singular and plural wording.

Walkthrough

The relative-time utility now supports month and year labels. New channel connections initialize scan boundaries at connection time. The dashboard adds tenant-scoped history analysis with selectable periods, scan-state resets, validation, authorization checks, and updated channel status data.

Changes

Relative-time display

Layer / File(s) Summary
Month and year formatting
src/lib/relative-time.ts, src/lib/relative-time.test.ts
relativeTime now returns month and year values beyond one week. Tests cover singular and plural labels.

Historical comment analysis

Layer / File(s) Summary
Connection-time scan boundary
src/lib/server/db/schema.ts, src/lib/server/channelConnect.ts, src/routes/api/auth/google/callback/callback.test.ts
New channel connections set cursor and createdAt from one timestamp. The schema documentation and callback test define and verify this scan boundary.
Dashboard history analysis flow
src/routes/(app)/dashboard/+page.server.ts, src/routes/(app)/dashboard/+page.svelte, src/routes/(app)/dashboard/actions.test.ts, src/routes/(app)/dashboard/dashboard.test.ts
The dashboard exposes lastRunAt and submits selectable history periods. The action validates periods, updates tenant-scoped scan state, resets pagination, and handles success, missing channels, unauthorized channels, and signed-out requests.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DashboardForm
  participant analyzeHistory
  participant ChannelsDatabase
  DashboardForm->>analyzeHistory: Submit selected history period
  analyzeHistory->>ChannelsDatabase: Validate tenant-scoped channel and update cursor
  ChannelsDatabase-->>analyzeHistory: Return updated scan state
  analyzeHistory-->>DashboardForm: Return success or validation/not-found error
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main changes to last-checked timestamps, connection history scanning, and opt-in history analysis.
Description check ✅ Passed The description directly explains the scan behavior, dashboard changes, validation, tests, and verification for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-last-checked-scan-window

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.

❤️ Share

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

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Aug 4, 2026
@codeant-ai

codeant-ai Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

🏁 CodeAnt Quality Gate Results

Commit: bcbaf4df
Scan Time: 2026-08-04 17:41:53 UTC

✅ Overall Status: PASSED

Quality Gate Details

Quality Gate Status Details
Secrets ✅ PASSED 0 secrets found
Duplicate Code ✅ PASSED 0.0% duplicated
SAST ✅ PASSED No security issues
Bugs ✅ PASSED Rating S: No bugs
IAC ✅ PASSED No IAC issues

View Full Results

@codeant-ai

codeant-ai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Sequence Diagram

This PR starts new channels at connection time and makes historical analysis an explicit bounded action. The scheduler then scans within the selected boundary, moderates only unseen comments, and records the actual run time used by the dashboard.

sequenceDiagram
    participant User
    participant Backend
    participant Dashboard
    participant Channel database
    participant Scheduler
    participant Comment API

    User->>Backend: Connect channel
    Backend->>Channel database: Create channel with connection time boundary

    User->>Dashboard: Request bounded history window
    Dashboard->>Channel database: Move boundary back and reset paging state

    Scheduler->>Comment API: Fetch comments within boundary
    Comment API-->>Scheduler: Return comment pages
    Scheduler->>Scheduler: Deduplicate and moderate unseen comments
    Scheduler->>Channel database: Save scan progress and actual run time
Loading

Generated by CodeAnt AI

@amazon-q-developer amazon-q-developer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed all code changes across 9 files. The implementation correctly addresses the issues described in the PR:

  • Bug fix: Dashboard now displays lastRunAt instead of cursor for "last checked", fixing the confusing display of old comment timestamps
  • Scan window changes: New channel connections initialize cursor to connection time (no historical analysis), while reconnects preserve existing cursor values
  • Analyze history feature: New server action with preset time windows (1/3/6/12/24 months) and proper validation, correctly bounded to control API costs
  • Time formatting: Added month/year units to relativeTime to prevent degradation to large week counts

The changes include proper tenancy scoping, validation, and error handling. All test files show comprehensive coverage of the new functionality. No blocking issues found.


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.

@codacy-production

codacy-production Bot commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

Not up to standards ⛔

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🔴 Metrics 18 complexity · 4 duplication

Metric Results
Complexity ✅ 18 (≤ 100 complexity)
Duplication ⚠️ 4 (≤ 1 duplication)

View in Codacy

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.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix dashboard “last checked” and add opt-in bounded history scan window

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Render dashboard “last checked” from channel lastRunAt (actual cron run time)
• Start new channel scan boundary at connect time; add explicit bounded “Analyze history” action
• Extend relative-time with month/year units and add coverage for new behaviors
Diagram

graph TD
  UI["Dashboard page"] --> Server["Dashboard load/actions"] --> DB[("channels table")]
  Connect["Channel connect upsert"] --> DB
  UI --> Util["relative-time"]
  DB --> Cron["Cron scanner"] --> DB
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Introduce a dedicated scanBoundary column (keep cursor as “newest seen”)
  • ➕ Reduces semantic overloading of cursor and avoids legacy confusion
  • ➕ Makes UI labels and cron logic clearer (boundary vs progress vs newest seen)
  • ➖ Requires schema migration and updates across cron and any existing cursor usage
  • ➖ More code churn vs the current minimal-change approach
2. Use calendar-month arithmetic (e.g., subMonths) instead of months*30 days
  • ➕ More intuitive boundaries (e.g., “3 months back” matches calendar expectations)
  • ➕ Avoids drift for long windows (12/24 months)
  • ➖ Adds dependency or custom date math complexity
  • ➖ Tests become slightly trickier around month ends and DST edge cases
3. Queue a background job for history analysis with progress/status
  • ➕ Clearer UX for long drains and safer rate limiting
  • ➕ Can surface progress and completion without relying on repeated cron runs
  • ➖ Introduces job infrastructure/state and more failure modes
  • ➖ Overkill if cron paging is already the chosen operational model

Recommendation: The PR’s approach is a good fit for the product constraints (bounded cost, explicit user intent, minimal new infrastructure). The main follow-up to consider is whether the “months = 30 days” approximation is acceptable for UX; if not, swap to calendar-month arithmetic later without changing the overall design.

Files changed (9) +145 / -7

Enhancement (3) +57 / -3
relative-time.tsAdd month/year buckets to relativeTime +5/-1

Add month/year buckets to relativeTime

• Introduces MONTH and YEAR thresholds and returns month/year units for larger diffs. Keeps weeks for <1 month to preserve existing behavior for recent history.

src/lib/relative-time.ts

+page.server.tsProject lastRunAt and add analyzeHistory action +32/-1

Project lastRunAt and add analyzeHistory action

• Dashboard load now selects lastRunAt for safe client rendering. Adds a tenancy-scoped analyzeHistory action that validates preset month windows, moves cursor back by N×30 days, and resets drain state (nextPageToken/scanCursor).

src/routes/(app)/dashboard/+page.server.ts

+page.svelteAdd Analyze history control and fix “last checked” display +20/-1

Add Analyze history control and fix “last checked” display

• Adds a per-channel POST form with a preset month selector to trigger history analysis. Switches the “last checked” UI to use lastRunAt instead of cursor and introduces minimal styling for the new inline form.

src/routes/(app)/dashboard/+page.svelte

Bug fix (1) +6 / -1
channelConnect.tsStart scan boundary at connection time for new channels +6/-1

Start scan boundary at connection time for new channels

• Sets cursor to the connection timestamp when inserting a new channel row. This prevents the initial cron scan from draining unlimited historical comments on first connect.

src/lib/server/channelConnect.ts

Tests (4) +81 / -2
relative-time.test.tsAdd month/year coverage for relativeTime +4/-0

Add month/year coverage for relativeTime

• Extends the test suite to validate month and year formatting, including singular/plural forms. Ensures older timestamps no longer degrade into very large week counts.

src/lib/relative-time.test.ts

actions.test.tsAdd analyzeHistory action tests (success + auth/validation/tenancy) +65/-0

Add analyzeHistory action tests (success + auth/validation/tenancy)

• Adds coverage ensuring analyzeHistory moves cursor back ~N×30 days and clears drain state. Verifies invalid month inputs return 400 without mutation, cross-tenant/unknown channels return 404, and signed-out requests are rejected with 401.

src/routes/(app)/dashboard/actions.test.ts

dashboard.test.tsAssert dashboard load includes lastRunAt and still omits secrets +8/-2

Assert dashboard load includes lastRunAt and still omits secrets

• Updates the dashboard load test to include lastRunAt in the returned channel payload. Retains the guarantee that encrypted refresh tokens are never serialized to the client.

src/routes/(app)/dashboard/dashboard.test.ts

callback.test.tsAssert new channel connect sets cursor=createdAt +4/-0

Assert new channel connect sets cursor=createdAt

• Extends the OAuth callback test to verify that new channel insertion initializes cursor to createdAt. Documents the intended behavior that pre-connect comments are not moderated unless history analysis is explicitly requested.

src/routes/api/auth/google/callback/callback.test.ts

Documentation (1) +1 / -1
schema.tsClarify cursor semantics as scan boundary +1/-1

Clarify cursor semantics as scan boundary

• Updates the schema comment to reflect cursor as a scan boundary rather than “newest comment seen,” documenting legacy null behavior and the new bounded-window model.

src/lib/server/db/schema.ts

@codeant-ai

codeant-ai Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

PR Code Suggestions ✨

Latest suggestions up to commit b163475
CategorySuggestion                                                                                                                                    SeverityGenerated at (UTC)
Race condition
An in-flight scan can overwrite a newly requested history boundary and drain state

The history-window reset is not coordinated with an in-flight cron run. A run can
snapshot the channel before this update, perform moderation work, and then
persistResults unconditionally write its stale cursor, nextPageToken, and scanCursor
afterward, overwriting the requested boundary or restoring the old continuation
token. Coordinate the reset with the channel lease or use an optimistic/versioned
update so stale runs cannot commit their scan state.

src/routes/(app)/dashboard/+page.server.ts [108-111]

Why it matters? 🤔
  • ❌ History analysis can lose its requested scan boundary.
  • ⚠️ Stale YouTube continuation state can skip or mis-scan history.
  • ⚠️ Dashboard users receive incorrect analyze-history results.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/routes/(app)/dashboard/+page.server.ts
**Line:** 108:111
**Comment:**
	*Race Condition: The history-window reset is not coordinated with an in-flight cron run. A run can snapshot the channel before this update, perform moderation work, and then `persistResults` unconditionally write its stale `cursor`, `nextPageToken`, and `scanCursor` afterward, overwriting the requested boundary or restoring the old continuation token. Coordinate the reset with the channel lease or use an optimistic/versioned update so stale runs cannot commit their scan state.

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
Major2026-08-04 17:11
Possible bug
History-action failures are rendered in an unrelated account-deletion error area

The history action form does not use use:enhance, so a failed analyzeHistory request
performs a full form navigation and places the returned form.error in the
account-deletion danger zone instead of next to the history control. Invalid windows
and not-found errors are therefore displayed detached from the action that caused
them and can be mistaken for deletion errors. Enhance this form and render its
errors in the channel card.

src/routes/(app)/dashboard/+page.svelte [107-117]

Why it matters? 🤔
  • ⚠️ History errors appear beside account deletion.
  • ⚠️ Users may misinterpret which action failed.
  • ⚠️ Invalid-window feedback requires a full page navigation.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/routes/(app)/dashboard/+page.svelte
**Line:** 107:117
**Comment:**
	*Possible Bug: The history action form does not use `use:enhance`, so a failed `analyzeHistory` request performs a full form navigation and places the returned `form.error` in the account-deletion danger zone instead of next to the history control. Invalid windows and not-found errors are therefore displayed detached from the action that caused them and can be mistaken for deletion errors. Enhance this form and render its errors in the channel card.

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
Major2026-08-04 17:11

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

👉 Steps to fix this

Actionable comments posted: 3

🤖 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 `@src/routes/`(app)/dashboard/+page.server.ts:
- Around line 93-115: Update the channels update in analyzeHistory to require
eq(channels.userId, user.id) alongside the existing orgId and channelId
predicates, preserving the 404 response when no row matches. Add a test proving
a same-org user cannot reset another user’s channel scan state.

In `@src/routes/`(app)/dashboard/+page.svelte:
- Around line 107-120: Update the history form around the analyzeHistory action
to use an operation-scoped result, rendering its error in an .error-box, success
status, and pending skeleton beside the form. Ensure the loading, empty, error,
and populated states required by the page are represented without reusing the
Delete account feedback state.

In `@src/routes/`(app)/dashboard/actions.test.ts:
- Around line 116-126: Update the rejected-request tests around analyzeHistory
to seed non-null values for every scan-state field, including nextPageToken and
scanCursor, then assert the complete scanWindowOf('UC1') object remains
unchanged after each 400 response. Apply the same full-state assertion to the
additional test case covering lines 135-143, preserving the existing cursor
value and rejection expectations.
🪄 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: 1daec4a7-58b2-4c1e-83ce-1c9ae348af7a

📥 Commits

Reviewing files that changed from the base of the PR and between 832dd3b and b163475.

📒 Files selected for processing (9)
  • src/lib/relative-time.test.ts
  • src/lib/relative-time.ts
  • src/lib/server/channelConnect.ts
  • src/lib/server/db/schema.ts
  • src/routes/(app)/dashboard/+page.server.ts
  • src/routes/(app)/dashboard/+page.svelte
  • src/routes/(app)/dashboard/actions.test.ts
  • src/routes/(app)/dashboard/dashboard.test.ts
  • src/routes/api/auth/google/callback/callback.test.ts

Comment on lines +93 to +115
analyzeHistory: async ({ request, locals }) => {
const user = requireUser(locals);
const f = await request.formData();
const channelId = String(f.get('channelId') ?? '');
const months = Number(f.get('months'));
// Preset windows only — the scan drains newest-first at 300 comments per
// run, so an unbounded window is an unbounded API/AI cost (I10).
if (![1, 3, 6, 12, 24].includes(months)) {
return fail(400, { error: 'history window must be 1, 3, 6, 12, or 24 months' });
}
// Move the scan boundary back and reset the drain state: cron's next runs
// page from the newest comment down to this boundary. Already-seen
// comments are skipped before scoring (decideNewComments dedupes by id),
// so only the unscanned history costs moderation calls. Tenancy-scoped:
// another team's channel reads as "not found".
const boundary = new Date(Date.now() - months * 30 * 24 * 60 * 60 * 1000).toISOString();
const updated = await db
.update(channels)
.set({ cursor: boundary, nextPageToken: null, scanCursor: null })
.where(and(eq(channels.id, channelId), eq(channels.orgId, user.orgId)))
.returning({ id: channels.id });
if (updated.length === 0) return fail(404, { error: 'channel not found' });
console.info(`history analysis requested for channel ${channelId}: scanning back to ${boundary}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== candidate file outline =="
ast-grep outline 'src/routes/(app)/dashboard/+page.server.ts' --view expanded || true

echo "== candidate file relevant sections =="
wc -l 'src/routes/(app)/dashboard/+page.server.ts'
sed -n '1,180p' 'src/routes/(app)/dashboard/+page.server.ts'

echo "== schema/model references for channels =="
rg -n "channels\.|eq\(channels|createTable.*channels|userId|orgId" src -g '*.ts' -g '*.tsx' | head -200

Repository: Bonobo791/Moderaty

Length of output: 27619


Authorization Bypass (CWE-862): Missing Authorization

Reachability: External

Scope analyzeHistory to the caller’s channel.

channelId is form input and the update here only checks channels.orgId === user.orgId, so any same-org user can reset another user’s channel scan state. Add eq(channels.userId, user.id) to the where, return 404 on no match, and add a same-org ownership test.

🤖 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/`(app)/dashboard/+page.server.ts around lines 93 - 115, Update the
channels update in analyzeHistory to require eq(channels.userId, user.id)
alongside the existing orgId and channelId predicates, preserving the 404
response when no row matches. Add a test proving a same-org user cannot reset
another user’s channel scan state.

Source: Coding guidelines

Comment on lines +107 to +120
<form method="POST" action="?/analyzeHistory" class="history-form">
<input type="hidden" name="channelId" value={ch.id} />
<label for="history-months-{ch.id}">Analyze history</label>
<select id="history-months-{ch.id}" name="months" aria-label="How far back to analyze comments on {ch.title}">
<option value="1">last month</option>
<option value="3" selected>last 3 months</option>
<option value="6">last 6 months</option>
<option value="12">last 12 months</option>
<option value="24">last 24 months</option>
</select>
<button class="btn secondary small" type="submit">Analyze history on {ch.title}</button>
</form>
<p class="muted" style="margin:12px 0 0">
last checked {ch.cursor ? relativeTime(ch.cursor) : 'never'} · ID: {ch.id}
last checked {ch.lastRunAt ? relativeTime(ch.lastRunAt) : 'never'} · ID: {ch.id}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Render history-action feedback in this card.

A failed analyzeHistory action sets form.error, but this page renders that error only in the Delete account card. The new form also has no success or pending state. Return an operation-scoped result and render its error, success status, and pending skeleton beside this form.

As per coding guidelines, “Invariant I12: Every page has all four states — loading (skeleton), empty (EmptyState component), error (.error-box), and populated.”

🤖 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/`(app)/dashboard/+page.svelte around lines 107 - 120, Update the
history form around the analyzeHistory action to use an operation-scoped result,
rendering its error in an .error-box, success status, and pending skeleton
beside the form. Ensure the loading, empty, error, and populated states required
by the page are represented without reusing the Delete account feedback state.

Source: Coding guidelines

Comment thread src/routes/(app)/dashboard/actions.test.ts
@qodo-code-review

qodo-code-review Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 78 rules

Grey Divider


Action required

1. AnalyzeHistory lost-update race ✓ Resolved 🐞 Bug ☼ Reliability
Description
The new analyzeHistory action updates cursor/nextPageToken/scanCursor without coordinating with the
cron lease, so an in-flight cron run can later persist its own scan state and overwrite the
user-requested history boundary. This can silently cancel (or partially apply) an analyze-history
request depending on which write commits last.
Code

src/routes/(app)/dashboard/+page.server.ts[R108-111]

+		const boundary = new Date(Date.now() - months * 30 * 24 * 60 * 60 * 1000).toISOString();
+		const updated = await db
+			.update(channels)
+			.set({ cursor: boundary, nextPageToken: null, scanCursor: null })
Relevance

●●● Strong

Team has accepted concurrency/race hardening before; likely to coordinate scan-state writes with
cron lease.

PR-#36
PR-#46

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Cron uses a lease to avoid concurrent cron runs, but the dashboard analyzeHistory action doesn’t
consult that lease and writes the same scan-state columns that the cron pipeline writes at the end
of a run; the pipeline write is unconditional on lease/org and can overwrite the dashboard’s update.

src/routes/(app)/dashboard/+page.server.ts[93-117]
src/routes/api/cron/+server.ts[77-112]
src/lib/server/pipeline.ts[470-494]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`analyzeHistory` mutates scan state (`cursor`, `nextPageToken`, `scanCursor`) while cron may concurrently run `runChannel()` and later call `persistResults()`, which writes the same fields. Because neither update is conditioned on the active lease, whichever finishes last overwrites the other.

## Issue Context
- Cron claims a per-channel lease via `channels.leaseExpiresAt`.
- `persistResults()` updates scan state by `channels.id` only.
- `analyzeHistory` updates scan state by `channels.id` + `channels.orgId` only.

## Fix Focus Areas
- src/routes/(app)/dashboard/+page.server.ts[93-117]
- src/routes/api/cron/+server.ts[77-112]
- src/lib/server/pipeline.ts[470-494]

## Recommended fix
Implement one of these coordination strategies:
1) **Reject while leased (simplest):** In `analyzeHistory`, first read the channel’s `leaseExpiresAt` (scoped by `id`+`orgId`), and if it’s non-null and in the future, return `fail(409, { error: 'channel is currently scanning; retry shortly' })`. Only perform the cursor reset when not leased.

2) **Optimistic concurrency:** Capture the current `leaseExpiresAt` (or a dedicated version column) and include it in the `WHERE` of the `UPDATE`. If 0 rows updated, return 409 and ask user to retry.

Optionally also harden `persistResults()` to only update when the current lease is still held by the same run (prevents stale writes after lease loss).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. analyzeHistory not scoped by userId 📘 Rule violation ⛨ Security
Description
The new analyzeHistory channel mutation scopes the update by channels.orgId but does not also
scope by channels.userId as required, allowing any user in the org to mutate a channel connected
by a different user. This can enable unauthorized history scans and associated API/AI cost on
another user’s connected channel.
Code

src/routes/(app)/dashboard/+page.server.ts[R109-112]

+		const updated = await db
+			.update(channels)
+			.set({ cursor: boundary, nextPageToken: null, scanCursor: null })
+			.where(and(eq(channels.id, channelId), eq(channels.orgId, user.orgId)))
Relevance

●●● Strong

Strong precedent for scoping channel access by channels.userId and returning 404 when not owned.

PR-#25

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2436659 requires channel mutations to be scoped to the authenticated user via a
channels.userId filter and to return 404 when not authorized. The new analyzeHistory action
updates channels with a WHERE clause that only checks channels.id and channels.orgId, omitting
channels.userId, despite channels.userId existing in the schema.

Rule 2436659: Scope channel operations to authenticated user and return 404 for unauthorized access
src/routes/(app)/dashboard/+page.server.ts[109-113]
src/lib/server/db/schema.ts[89-96]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `analyzeHistory` action updates `channels.cursor` without scoping the mutation by `channels.userId` (only by `channels.orgId`), which violates the requirement that channel operations be scoped to the authenticated user and return 404 for unauthorized access.

## Issue Context
`channels` includes both `userId` and `orgId`. The compliance requirement explicitly calls for scoping channel mutations by `channels.userId` to prevent a user from operating on another user's channel.

## Fix Focus Areas
- src/routes/(app)/dashboard/+page.server.ts[109-113]
- src/lib/server/db/schema.ts[89-96]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Backfill delays new comments 🐞 Bug ≡ Correctness
Description
analyzeHistory can initiate a multi-run historical drain that stores nextPageToken, and subsequent
cron runs will continue from that continuation token rather than restarting at the newest page. This
provides no guarantee that comments posted after the backfill begins will be moderated promptly
(they may be delayed until the drain completes and pagination resets).
Code

src/routes/(app)/dashboard/+page.server.ts[R108-111]

+		const boundary = new Date(Date.now() - months * 30 * 24 * 60 * 60 * 1000).toISOString();
+		const updated = await db
+			.update(channels)
+			.set({ cursor: boundary, nextPageToken: null, scanCursor: null })
Relevance

●● Moderate

Product/latency tradeoff; no clear precedent on prioritizing newest comments during history drain.

PR-#3

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The action sets a far-back cursor and clears paging state; the pipeline persists and reuses
nextPageToken across runs, and fetchNewComments uses pageToken to start listing from that point, so
the system may keep paging older history instead of repeatedly checking newest comments during the
drain.

src/routes/(app)/dashboard/+page.server.ts[93-117]
src/lib/server/pipeline.ts[470-494]
src/lib/server/pipeline.ts[505-526]
src/lib/server/youtube.ts[223-264]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When `analyzeHistory` moves `cursor` far back, `fetchNewComments()` may return `nextPageToken` for large windows. While `channels.nextPageToken` is non-null, `runChannel()` passes it back into `fetchNewComments()`, so the scan resumes mid-history instead of re-checking newest comments first.

## Issue Context
- `analyzeHistory` resets to a far-back cursor and clears tokens to start a drain.
- `persistResults()` stores `nextPageToken` + a fixed `scanCursor` to continue the drain in later cron runs.
- `runChannel()` always supplies `pageToken: channel.nextPageToken`.

## Fix Focus Areas
- src/routes/(app)/dashboard/+page.server.ts[93-117]
- src/lib/server/pipeline.ts[470-494]
- src/lib/server/pipeline.ts[505-559]
- src/lib/server/youtube.ts[223-264]

## Recommended fix
Add a “live-first” behavior during long drains, for example:
- If `channel.nextPageToken` is set (drain in progress), **first** fetch/process page(s) from the newest end to catch up on new comments since `channel.scanCursor` (or since the current high-water), **then** continue the historical drain using `nextPageToken`.

This may require storing separate state for the drain (e.g., `historyNextPageToken` / `historyBoundary`) so the live cursor can keep advancing without aborting the backfill. Ensure dedupe remains in place so comments aren’t re-scored.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment on lines +109 to +112
const updated = await db
.update(channels)
.set({ cursor: boundary, nextPageToken: null, scanCursor: null })
.where(and(eq(channels.id, channelId), eq(channels.orgId, user.orgId)))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. analyzehistory not scoped by userid 📘 Rule violation ⛨ Security

The new analyzeHistory channel mutation scopes the update by channels.orgId but does not also
scope by channels.userId as required, allowing any user in the org to mutate a channel connected
by a different user. This can enable unauthorized history scans and associated API/AI cost on
another user’s connected channel.
Agent Prompt
## Issue description
The `analyzeHistory` action updates `channels.cursor` without scoping the mutation by `channels.userId` (only by `channels.orgId`), which violates the requirement that channel operations be scoped to the authenticated user and return 404 for unauthorized access.

## Issue Context
`channels` includes both `userId` and `orgId`. The compliance requirement explicitly calls for scoping channel mutations by `channels.userId` to prevent a user from operating on another user's channel.

## Fix Focus Areas
- src/routes/(app)/dashboard/+page.server.ts[109-113]
- src/lib/server/db/schema.ts[89-96]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread src/routes/(app)/dashboard/+page.server.ts
Comment on lines +108 to +111
const boundary = new Date(Date.now() - months * 30 * 24 * 60 * 60 * 1000).toISOString();
const updated = await db
.update(channels)
.set({ cursor: boundary, nextPageToken: null, scanCursor: null })

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

3. Backfill delays new comments 🐞 Bug ≡ Correctness

analyzeHistory can initiate a multi-run historical drain that stores nextPageToken, and subsequent
cron runs will continue from that continuation token rather than restarting at the newest page. This
provides no guarantee that comments posted after the backfill begins will be moderated promptly
(they may be delayed until the drain completes and pagination resets).
Agent Prompt
## Issue description
When `analyzeHistory` moves `cursor` far back, `fetchNewComments()` may return `nextPageToken` for large windows. While `channels.nextPageToken` is non-null, `runChannel()` passes it back into `fetchNewComments()`, so the scan resumes mid-history instead of re-checking newest comments first.

## Issue Context
- `analyzeHistory` resets to a far-back cursor and clears tokens to start a drain.
- `persistResults()` stores `nextPageToken` + a fixed `scanCursor` to continue the drain in later cron runs.
- `runChannel()` always supplies `pageToken: channel.nextPageToken`.

## Fix Focus Areas
- src/routes/(app)/dashboard/+page.server.ts[93-117]
- src/lib/server/pipeline.ts[470-494]
- src/lib/server/pipeline.ts[505-559]
- src/lib/server/youtube.ts[223-264]

## Recommended fix
Add a “live-first” behavior during long drains, for example:
- If `channel.nextPageToken` is set (drain in progress), **first** fetch/process page(s) from the newest end to catch up on new comments since `channel.scanCursor` (or since the current high-water), **then** continue the historical drain using `nextPageToken`.

This may require storing separate state for the drain (e.g., `historyNextPageToken` / `historyBoundary`) so the live cursor can keep advancing without aborting the backfill. Ensure dedupe remains in place so comments aren’t re-scored.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@codeant-ai codeant-ai Bot added size:L This PR changes 100-499 lines, ignoring generated files and removed size:L This PR changes 100-499 lines, ignoring generated files labels Aug 4, 2026
@Bonobo791

Copy link
Copy Markdown
Collaborator Author

Review triage complete — fixes pushed in bcbaf4d:

Fixed (valid findings):

  1. Lost-update race (Qodo) — analyzeHistory now does an atomic UPDATE that includes the lease-claim predicate (leaseExpiresAt IS NULL OR < now), mirroring the cron claim. A channel mid-scan returns 409 'This channel is mid-scan — retry in a minute.' and leaves cursor/nextPageToken/scanCursor untouched, so an in-flight persistResults() can no longer overwrite the user's reset boundary with a stale one. Regression tests: leased channel → 409 with full scan state unchanged; expired lease → success.
  2. Feedback rendered in the wrong card (CodeRabbit) — action results now carry scope: 'history' + channelId; the per-channel card renders its own error/success next to the form, and the Delete-account card ignores history-scoped results.
  3. Test hardening (CodeRabbit) — rejection tests now seed nextPageToken/scanCursor and assert the full scan-state object via toEqual, so a regression that clobbers the drain checkpoint fails the suite.

Not a bug (skipped with reason):

  • Scope analyzeHistory by userId too (CodeRabbit/Qodo) — contradicts the merged team model: channels belong to the org, and any teammate may operate them. Precedent: setToneLevel scopes by orgId only, and there's an explicit test that a teammate's channel must appear and be operable. Scoping by userId would break the team flow.

Acknowledged, follow-up filed:

  • Long history drains delay brand-new comments (Qodo) — valid, accepted v1 tradeoff. The proper fix (separate drain state columns + live-first pipeline) is a schema change in the DB engineer's layer → Live-first scanning during long history drains #70.

Gate: npm run test 422/422, npm run check clean, npm run build green.

@sonarqubecloud

sonarqubecloud Bot commented Aug 4, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant