Skip to content

fix(watchers): malformed array literal swallows chat replies#994

Merged
buremba merged 1 commit into
mainfrom
feat/fix-watcher-run-array
May 21, 2026
Merged

fix(watchers): malformed array literal swallows chat replies#994
buremba merged 1 commit into
mainfrom
feat/fix-watcher-run-array

Conversation

@buremba
Copy link
Copy Markdown
Member

@buremba buremba commented May 20, 2026

Problem

resolveWatcherRunsByMessageIds (run on every terminal chat payload by api-response-renderer) built dispatched_message_id = ANY(${ids}::text[]) from a raw JS array. The bundled postgres.js serializes that parameter as a bare scalar (the message UUID), so Postgres rejects it:

ERROR: malformed array literal: "3b50ef02-64f5-47c1-bcdc-d93b04e8a69f"
  detail: Array value must start with "{"   where: $1 = '<uuid>'
[api-response-renderer] Failed to resolve watcher runs from terminal API payload

Because it throws in the response-render path, it intermittently swallows the final CLI render of an otherwise-successful chat. Every other call site in the codebase uses the canonical ANY(${pg*Array(...)}::type[]) (string-literal helper + cast); this was the lone raw-array usage.

Fix

Wrap ids with pgTextArray(ids) (already exported from db/client.ts), matching runStatusLiteral/pgBigintArray usage everywhere else. 2-line change.

Reproducer (red → green, real bundled server, embedded Postgres)

  • RED (unfixed main build): a lobu chat turn logs malformed array literal: "<uuid>" + Failed to resolve watcher runs from terminal API payload (Postgres 22P02).
  • GREEN (this branch): same chat → 0 such errors; Session completed successfully - all content already streamed.

Note: a standalone postgres.js repro did not reproduce it (it serialized the JS array correctly) — the bug only manifests through the esbuild-bundled server, which is why the E2E was run against the actual lobu run build.

Summary by CodeRabbit

  • Refactor
    • Enhanced database query parameterization for watcher run operations to improve consistency and reliability.

Review Change Stack

resolveWatcherRunsByMessageIds passed a raw JS array as `${ids}::text[]`.
The bundled postgres.js serializes that as a bare scalar (the message UUID),
so Postgres rejects it: malformed array literal: "<uuid>". This throws on
every terminal chat payload (api-response-renderer 'Failed to resolve watcher
runs from terminal API payload'), intermittently swallowing the final CLI
render. Wrap with pgTextArray(ids) to match the canonical
ANY(${pg*Array(...)}::type[]) pattern used at every other call site.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5b896b58-99d4-4e8c-85d5-38da4a118fe2

📥 Commits

Reviewing files that changed from the base of the PR and between 86b3f17 and a2f5177.

📒 Files selected for processing (1)
  • packages/server/src/watchers/run-completion.ts

📝 Walkthrough

Walkthrough

The PR updates run-completion.ts to use a pgTextArray helper function when binding message ID arrays to Postgres queries, replacing direct array parameterization in the resolveWatcherRunsByMessageIds function's dispatched_message_id filter.

Changes

Postgres Array Binding Refactor

Layer / File(s) Summary
Postgres array binding refactor
packages/server/src/watchers/run-completion.ts
Add pgTextArray import from ../db/client and apply it to the dispatched_message_id filter, changing from ids::text[] to pgTextArray(ids)::text[].

Possibly related PRs

  • lobu-ai/lobu#835: Both PRs adopt safer Postgres array parameterization; this PR updates dispatched_message_id text array binding while the related PR addresses UUID array handling.

Poem

🐰 Arrays bound tight with careful hands,
pgTextArray now understands,
Safe from SQL's twisted tricks,
Two lines dance, the code fix clicks!

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Description check ❓ Inconclusive The description includes a clear problem statement with error details and reproducer steps, the 2-line fix explanation, and E2E validation results; however, the Test plan checkbox section from the template is entirely missing. Add the Test plan section with checkboxes for validation steps (bun run check:fix, typecheck, etc.) and confirm which tests were actually executed.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main fix: wrapping a raw array parameter with pgTextArray to prevent malformed array literal errors that were swallowing chat replies.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/fix-watcher-run-array

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

@buremba
Copy link
Copy Markdown
Member Author

buremba commented May 20, 2026

bug_free 86, simplicity 100, slop 0, bugs 0, 0 blockers

Typecheck/unit passed. [env] Integration failed because harness refused database "postgres" as non-test DB, unrelated to changed watcher file. Explored pgTextArray via bun -e for representative ids/escaping; no live DB probe because DATABASE_URL/psql unavailable. DB-only query holds with 3 replicas.

Full verdict JSON
{
  "bug_free_confidence": 86,
  "bugs": 0,
  "slop": 0,
  "simplicity": 100,
  "blockers": [],
  "change_type": "fix",
  "behavior_change_risk": "low",
  "tests_adequate": false,
  "suggested_fixes": [],
  "notes": "Typecheck/unit passed. [env] Integration failed because harness refused database \"postgres\" as non-test DB, unrelated to changed watcher file. Explored pgTextArray via bun -e for representative ids/escaping; no live DB probe because DATABASE_URL/psql unavailable. DB-only query holds with 3 replicas.",
  "categories": {
    "src": 4,
    "tests": 0,
    "docs": 0,
    "config": 0,
    "deps": 0,
    "migrations": 0,
    "ci": 0,
    "generated": 0
  }
}

Local review gate — branch protection can require the pi-review commit status. See docs/REVIEW_SCHEMA.md.

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@buremba buremba merged commit 9734aca into main May 21, 2026
16 of 18 checks passed
@buremba buremba deleted the feat/fix-watcher-run-array branch May 21, 2026 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants