fix(plugin): read channel self get from replica

[riderx]

Summary (AI generated)

• Route old-plugin /channel_self PUT read requests through the read-replica Postgres client.
• Keep POST and DELETE on the primary connection because they can write channel_devices.

Motivation (AI generated)

Supabase reported plugin workers still opening direct primary Postgres sessions. The legacy /channel_self PUT handler is a read action, so it should use the existing read-replica path instead of the write/default connection.

Business Impact (AI generated)

This reduces primary database connection pressure for old plugin channel reads without changing public API responses or legacy write behavior.

Test Plan (AI generated)

• bun lint:backend
• bun typecheck
• bun run supabase:with-env -- bunx vitest run tests/channel_self.test.ts --fileParallelism=false --hookTimeout=60000

Generated with AI

Summary by CodeRabbit

• Chores
  • Updated PostgreSQL client configuration for channel self operations endpoint.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[coderabbitai]

Note

Reviews paused

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

Use the following commands to manage reviews:

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

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR modifies PostgreSQL client initialization in the channel self PUT endpoint by adding a true flag to getPgClient(c, true). This single-line change alters which database instance (primary or read-replica) the endpoint uses for queries.

Changes

Channel Self PG Client Configuration

Layer / File(s)	Summary
PG client replica selection supabase/functions/_backend/plugins/channel_self.ts	The app.put('/') handler now initializes the PG client with getPgClient(c, true) instead of the default getPgClient(c), controlling database instance routing for PUT requests.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• Cap-go/capgo#2245: Both PRs adjust the getPgClient() flag to control endpoint routing between read-replica and primary database instances.

Poem

A rabbit hops with purpose true,
One flag flips—now primary reads through,
Channel self knows where to go,
Small change, but the data will flow! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.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
Title check	✅ Passed	The title accurately describes the main change: routing the channel_self read operation through the read-replica database instead of the primary connection.
Description check	✅ Passed	The description includes a clear summary, motivation, and test plan. While formatted differently than the template, it covers the key required sections with sufficient detail about the change rationale and verification steps.
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.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/channel-self-read-replica

---

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

[codspeed-hq]

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks¹

---

_{Comparing codex/channel-self-read-replica (a6cb494) with main (b1f80da)²}

Footnotes

1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

2. No successful run was found on main (adbe75f) during the generation of this report, so b1f80da was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[coderabbitai]

🧹 Nitpick comments (2)

tests/files-app-read-guard.unit.test.ts (1)

37-37: ⚡ Quick win

Use it.concurrent() for the updated test cases.

Both changed tests should use it.concurrent() to match the suite’s parallel-execution requirement.

♻️ Suggested change

-  it('serves cached app-scoped files without app lookup', async () => {
+  it.concurrent('serves cached app-scoped files without app lookup', async () => {

-  it('serves cached malformed app-scoped paths without app lookup', async () => {
+  it.concurrent('serves cached malformed app-scoped paths without app lookup', async () => {

As per coding guidelines "tests/**/*.test.ts: Design all tests for parallel execution across files; use it.concurrent() instead of it() to run tests in parallel within the same file for faster CI/CD".

Also applies to: 75-75

🤖 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 `@tests/files-app-read-guard.unit.test.ts` at line 37, Replace the two
synchronous tests that use it(...) with parallel execution by changing them to
it.concurrent(...); specifically update the test whose title is "serves cached
app-scoped files without app lookup" and the other modified test at the second
location (line referenced in comment) to call it.concurrent instead of it so
they run in parallel with the suite's requirements.

tests/files-security.test.ts (1)

220-220: ⚡ Quick win

Use it.concurrent() for this updated test case.

Switch this updated test to it.concurrent() to keep the file aligned with parallel-test policy.

♻️ Suggested change

-  it('continues serving uploaded attachments while the storage object still exists after the app is deleted', async () => {
+  it.concurrent('continues serving uploaded attachments while the storage object still exists after the app is deleted', async () => {

As per coding guidelines "tests/**/*.test.ts: Design all tests for parallel execution across files; use it.concurrent() instead of it() to run tests in parallel within the same file for faster CI/CD".

🤖 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 `@tests/files-security.test.ts` at line 220, The test case "continues serving
uploaded attachments while the storage object still exists after the app is
deleted" uses it(...) but should use it.concurrent(...) to enable parallel
execution; update the test invocation in tests/files-security.test.ts by
replacing the it(...) call wrapping that async test with it.concurrent(...),
preserving the async function body and any timers/timeouts or shared fixtures so
the test remains safe to run in parallel.

🤖 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.

Nitpick comments:
In `@tests/files-app-read-guard.unit.test.ts`:
- Line 37: Replace the two synchronous tests that use it(...) with parallel
execution by changing them to it.concurrent(...); specifically update the test
whose title is "serves cached app-scoped files without app lookup" and the other
modified test at the second location (line referenced in comment) to call
it.concurrent instead of it so they run in parallel with the suite's
requirements.

In `@tests/files-security.test.ts`:
- Line 220: The test case "continues serving uploaded attachments while the
storage object still exists after the app is deleted" uses it(...) but should
use it.concurrent(...) to enable parallel execution; update the test invocation
in tests/files-security.test.ts by replacing the it(...) call wrapping that
async test with it.concurrent(...), preserving the async function body and any
timers/timeouts or shared fixtures so the test remains safe to run in parallel.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 373c8c29-5a9f-4e79-a96e-1494eb65154a

📥 Commits

Reviewing files that changed from the base of the PR and between 27b215d and 68a6a8c.

📒 Files selected for processing (4)

• supabase/functions/_backend/files/files.ts
• tests/files-app-read-guard.unit.test.ts
• tests/files-local-read-proxy.unit.test.ts
• tests/files-security.test.ts

✅ Files skipped from review due to trivial changes (1)

• supabase/functions/_backend/files/files.ts

[digzrow-coder]

I think this should stay on the primary for the legacy override path, or split the handler so only the local-storage/new-plugin branch can use the replica.

POST /channel_self writes/removes channel_devices on the primary for older clients, and those clients commonly call PUT /channel_self right after setting the channel to resolve the effective channel. With this PR, PUT now uses getPgClient(c, true), but put() still reads getChannelDeviceOverridePg() and getChannelsPg() from that client for the pre-vX.34.0 path.

That makes the immediate read-after-write path replica-lag dependent: a device can successfully self-assign to a channel, then its next PUT can miss the just-written override and fall back to the public/default channel. During staged rollouts this can send the device back to production or the wrong channel until replication catches up.

A narrow fix would be to keep PUT on the primary when isChannelSelfLocalChannelStorageVersion(c, body, 'PUT') is false, and only use the read replica for the new local-storage path/listing reads that do not depend on channel_devices read-your-write consistency.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud