Refactor repository for VLM chat application - #1
Conversation
- Rename project to vlm-chat and update documentation - Replace greeting feature with chat conversation system - Add local SQLite database migration for conversations and attachments - Implement inference client for MLX VLM and OpenAI-compatible API - Add chat UI with attachment handling and streaming support - Replace Turso management scripts with local inference server management - Update integration and system tests for the new architecture
📝 WalkthroughWalkthroughThis change replaces the starter application with a SQLite-backed VLM chat client. It adds MLX inference tooling, conversation and attachment persistence, streaming APIs, image handling, cancellation, a chat interface, and integration/system test infrastructure. ChangesVLM Chat
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🟠 High · up to The chat flow can corrupt active completion tracking across conversations and can create duplicate conversations when assistant-message creation fails and the user retries. These are concrete correctness and data-integrity risks, so the PR is not ready to merge until the failure paths and completion ownership are fixed. Sequence Diagram(s)sequenceDiagram
participant User
participant ChatView
participant ChatRoute
participant Prisma
participant InferenceServer
User->>ChatView: Enter text or attach images
ChatView->>ChatRoute: POST completion request
ChatRoute->>Prisma: Persist user message
ChatRoute->>InferenceServer: Open streaming completion
InferenceServer-->>ChatRoute: SSE completion chunks
ChatRoute-->>ChatView: Stream chat events
ChatRoute->>Prisma: Persist assistant outcome
ChatView->>ChatRoute: DELETE completion on stop
ChatRoute-->>ChatView: Return cancellation result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (6)
tests/integration/chat-route.test.ts (1)
69-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore
VLM_CHAT_INFERENCE_URLafter each test. Both files overwrite the process environment and never restore the previous value. Bun runs test files in a shared process, so a stale endpoint leaks into later tests. After a stub stops, its port is released, and a later test that forgets to set the variable can point at a dead or reused port. This produces order-dependent failures.
tests/integration/chat-route.test.ts#L69-L75: capture the previous value inuseStuband restore it in thestopfunction, so every existingfinally { stub.stop(); }block also resets the environment.tests/integration/models-route.test.ts#L10-L45: replace the two inline assignments with the same fixture helper, and stop the stub in afinallyblock in the second test as well.♻️ Proposed shared helper
Add the environment handling to the fixture in
tests/integration/fixtures/inference-stub.ts:export function startInferenceStub(script: InferenceStubScript = {}) { const encoder = new TextEncoder(); const received: unknown[] = []; + const previousEndpoint = process.env.VLM_CHAT_INFERENCE_URL; @@ return { url: `http://127.0.0.1:${server.port}`, received, - stop: () => server.stop(true), + stop: () => { + if (previousEndpoint === undefined) { + delete process.env.VLM_CHAT_INFERENCE_URL; + } else { + process.env.VLM_CHAT_INFERENCE_URL = previousEndpoint; + } + + return server.stop(true); + }, }; }🤖 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/integration/chat-route.test.ts` around lines 69 - 75, Update tests/integration/chat-route.test.ts lines 69-75 by making useStub capture the prior VLM_CHAT_INFERENCE_URL and restore it when the returned stub’s stop function runs. Update tests/integration/models-route.test.ts lines 10-45 to use the same fixture helper instead of inline environment assignments, and ensure the second test stops its stub in a finally block.src/lib/inference/client.ts (1)
51-62: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the model-list request with a timeout.
If the inference server accepts the connection but never responds,
/api/modelswaits indefinitely. Add a 5-second deadline tofetchModelIdswhile preserving the caller’s abort signal. Keep streaming completions without a deadline.♻️ Proposed timeout for the model list
export async function fetchModelIds(signal?: AbortSignal) { - const { response, endpoint } = await request('/v1/models', { signal }); + const { response, endpoint } = await request('/v1/models', { + signal: signal + ? AbortSignal.any([signal, AbortSignal.timeout(5_000)]) + : AbortSignal.timeout(5_000), + }); const parsed = modelListSchema.safeParse(await response.json());🤖 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/inference/client.ts` around lines 51 - 62, Update fetchModelIds so its /v1/models request uses a 5-second timeout while still honoring the caller-provided AbortSignal. Apply the deadline only to this model-list request and leave streaming completion requests unchanged.src/app/_components/chat-view.tsx (2)
58-92: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAbort the in-flight completion when the component unmounts.
abortholds the completion controller, but no cleanup aborts it. The parent page keysChatViewby conversation id (src/app/conversations/[conversationId]/page.tsxLine 34), so navigating to another conversation during a stream unmounts this component and leaves the reader consuming the response body. Add an unmount effect.♻️ Proposed cleanup effect
const abort = useRef<AbortController | null>(null); + + useEffect(() => () => abort.current?.abort(), []);🤖 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/app/_components/chat-view.tsx` around lines 58 - 92, Add an unmount cleanup effect in ChatView that calls abort.current?.abort() so an active completion is cancelled when the conversation changes or the component unmounts. Keep the existing abort-controller handling and avoid affecting normal completion.
114-128: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winEnforce the attachment cap inside the state updater.
roomreads theimagesvalue captured by the current render. TwoaddFilescalls that start before a re-render (for example a paste followed by a drop) each compute the sameroom, so the combined result can exceedMAX_ATTACHMENTS. The server then rejects the send instead of showing the message on Line 118. Clamp inside the functional update.♻️ Proposed refactor
try { const added = await Promise.all(files.slice(0, room).map(downscaleImage)); - setImages((current) => [...current, ...added]); + setImages((current) => + [...current, ...added].slice(0, MAX_ATTACHMENTS), + ); } catch (cause) { setError(describe(cause)); }🤖 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/app/_components/chat-view.tsx` around lines 114 - 128, Update addFiles so the functional setImages updater enforces MAX_ATTACHMENTS using the latest current state, rather than relying on the render-captured room value; preserve the existing downscaling and error handling while clamping added images to the remaining capacity.src/app/_components/composer.tsx (1)
40-44: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFilter dropped and pasted files by the accepted image types.
ACCEPTEDon Line 6 constrains only the hidden file input. A dropped or pasted PDF reachesdownscaleImageand produces a decode error in the banner. Filter with the same list.♻️ Proposed refactor
-const ACCEPTED = 'image/png,image/jpeg,image/webp'; +const ACCEPTED_TYPES = ['image/png', 'image/jpeg', 'image/webp']; +const ACCEPTED = ACCEPTED_TYPES.join(','); + +function imagesOnly(files: File[]) { + return files.filter((file) => ACCEPTED_TYPES.includes(file.type)); +}function handleDrop(event: DragEvent) { event.preventDefault(); setDragging(false); - onFiles(Array.from(event.dataTransfer.files)); + onFiles(imagesOnly(Array.from(event.dataTransfer.files))); }Apply the same filter to the paste handler on Lines 67-74.
🤖 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/app/_components/composer.tsx` around lines 40 - 44, Update handleDrop and the paste handler to filter files against the existing ACCEPTED image types before calling onFiles. Reuse the same acceptance check for both dropped and pasted files so unsupported files such as PDFs never reach downscaleImage.src/app/api/attachments/[attachmentId]/route.ts (1)
19-26: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueAdd
X-Content-Type-Options: nosniffas defense in depth
parseCompletionRequestrestricts the only attachment write path toimage/png,image/jpeg, andimage/webp. Client input therefore cannot storetext/html; this is not a major stored-XSS issue. Addingnosniffremains an optional hardening measure.🤖 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/app/api/attachments/`[attachmentId]/route.ts around lines 19 - 26, Add the X-Content-Type-Options response header with the value nosniff in the attachment route’s Response headers alongside Content-Type and Cache-Control.
🤖 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 @.github/workflows/run-react-doctor.yml:
- Line 33: Add react-doctor as an exact dev dependency so it is recorded in
package.json and bun.lock, then update the workflow command to invoke the
project-installed binary without the `@latest` suffix. Preserve the existing
arguments and GitHub summary output behavior.
In `@inference/justfile`:
- Around line 48-56: Update the ask recipe to construct its JSON request body
with jq --arg or an equivalent serializer so prompt and model values safely
handle quotes, apostrophes, and newlines. Shell-quote every interpolated recipe
parameter in both ask and download, including endpoint, model, and prompt, while
preserving the existing request and download behavior.
In `@playwright.config.ts`:
- Around line 45-53: Update the Playwright webServer configuration to set a
dedicated test DATABASE_URL alongside VLM_CHAT_INFERENCE_URL, ensuring db:setup
and the application use the isolated database. Prevent reuseExistingServer from
bypassing this configuration, or add validation that the reused server matches
both the dedicated database and stubInferenceUrl settings.
In `@prisma/schema.prisma`:
- Around line 17-20: Update the transaction paths used by startAssistantMessage
and closeAssistantMessage to also touch the parent Conversation.updatedAt when
creating or completing an assistant message. Ensure each parent update occurs in
the same transaction as its corresponding message operation so listConversations
ordering reflects assistant activity.
In `@README.md`:
- Line 12: Update the fenced diagram block in README.md to specify the text
language identifier, changing the unannotated fence to a text-labeled fence so
markdownlint MD040 passes.
In `@src/app/_components/attachment-tray.tsx`:
- Around line 16-17: Prevent duplicate React keys in both image lists: update
src/app/_components/attachment-tray.tsx lines 16-17 to derive the key from
image.dataUrl and the map index, and update src/app/_components/turn.tsx lines
66-67 to derive the key from source and the map index. Preserve the existing
rendering behavior while ensuring duplicate image data URLs receive distinct
keys.
In `@src/app/_components/conversation-list.tsx`:
- Around line 19-35: Update the remove function to catch rejected fetch or
response-processing errors and pass a user-facing message to the existing
setError/ErrorBanner flow. Preserve the current handling for non-OK responses
and successful active/non-active conversation behavior.
In `@src/app/api/chat/route.ts`:
- Around line 92-98: Wrap the await of startAssistantMessage in the route
handler with failure handling that calls generation.abort() and consumes or
otherwise closes upstream before propagating the error. Keep
registerCompletion(assistantMessage.id, generation) on the successful path so
the upstream generation is always terminated when assistant-message creation
fails.
In `@src/features/completion/history.ts`:
- Around line 18-42: Update the history-loading flow to avoid selecting or
base64-encoding attachment data for older turns. In the query used by the
surrounding history function, fetch attachment identifiers or counts instead of
data, identify the newest user message, then load attachment data only for that
message and preserve placeholder entries for older attachments so
buildCompletionMessages retains the correct image-count notice.
In `@src/features/completion/parse.ts`:
- Around line 22-30: Update the dataBase64 validation chain in the completion
parser to reject values whose length is not divisible by 4, while preserving the
existing non-empty, character, and byte-size checks. Add the length validation
alongside the existing BASE64 refinement so malformed payloads cannot reach
image decoding.
In `@src/features/completion/run.ts`:
- Around line 87-93: Ensure completion cleanup does not depend solely on the
generator finally block: in src/features/completion/run.ts lines 87-93, expose
or accept a settle-and-release cleanup callback that the route can invoke when
the stream is abandoned before iteration begins, while preserving the existing
generator cleanup. In src/lib/running-completions.ts lines 10-17, make the
completion registry self-cleaning by recording registration time and removing or
aborting unreleased entries after a bounded timeout.
In `@src/lib/abort-error.ts`:
- Around line 1-6: Update stream handling around decodeCompletionStream and
runCompletion to accept the relevant AbortSignal, classify signal.aborted as an
aborted outcome before emitting incomplete-stream failures, and preserve
existing cause-chain detection for wrapped AbortError or code ABORT_ERR
rejections. Add a regression test covering Bun 1.3.14 behavior where an aborted
stream ends with done: true.
In `@src/lib/inference/schema.ts`:
- Around line 42-45: Update the completion chunk schema near finish_reason so
the choices collection is required by removing its .default([]) fallback. Ensure
decodeCompletionStream rejects payloads that omit choices while preserving
validation for valid chunks.
In `@tests/integration/conversation-database.test.ts`:
- Line 89: Update the ordering assertion in the conversation listing test to
first verify that both older and newer conversation IDs are present in listed,
then compare their indexes. Keep the existing ordering check after these
presence assertions so a missing ID cannot produce a false pass.
---
Nitpick comments:
In `@src/app/_components/chat-view.tsx`:
- Around line 58-92: Add an unmount cleanup effect in ChatView that calls
abort.current?.abort() so an active completion is cancelled when the
conversation changes or the component unmounts. Keep the existing
abort-controller handling and avoid affecting normal completion.
- Around line 114-128: Update addFiles so the functional setImages updater
enforces MAX_ATTACHMENTS using the latest current state, rather than relying on
the render-captured room value; preserve the existing downscaling and error
handling while clamping added images to the remaining capacity.
In `@src/app/_components/composer.tsx`:
- Around line 40-44: Update handleDrop and the paste handler to filter files
against the existing ACCEPTED image types before calling onFiles. Reuse the same
acceptance check for both dropped and pasted files so unsupported files such as
PDFs never reach downscaleImage.
In `@src/app/api/attachments/`[attachmentId]/route.ts:
- Around line 19-26: Add the X-Content-Type-Options response header with the
value nosniff in the attachment route’s Response headers alongside Content-Type
and Cache-Control.
In `@src/lib/inference/client.ts`:
- Around line 51-62: Update fetchModelIds so its /v1/models request uses a
5-second timeout while still honoring the caller-provided AbortSignal. Apply the
deadline only to this model-list request and leave streaming completion requests
unchanged.
In `@tests/integration/chat-route.test.ts`:
- Around line 69-75: Update tests/integration/chat-route.test.ts lines 69-75 by
making useStub capture the prior VLM_CHAT_INFERENCE_URL and restore it when the
returned stub’s stop function runs. Update
tests/integration/models-route.test.ts lines 10-45 to use the same fixture
helper instead of inline environment assignments, and ensure the second test
stops its stub in a finally block.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0fa3e0ed-ac06-432b-986f-f5801f108f6f
⛔ Files ignored due to path filters (3)
inference/uv.lockis excluded by!**/*.lockpublic/globe.svgis excluded by!**/*.svgtests/system/fixtures/pixel.pngis excluded by!**/*.png
📒 Files selected for processing (97)
.github/workflows/run-react-doctor.yml.github/workflows/run-system-tests.yml.gitignoreAGENTS.mdREADME.mdbiome.jsondoctor.config.jsoninference/.python-versioninference/justfileinference/pyproject.tomlpackage.jsonplaywright.config.tsprisma.config.tsprisma/migrations/20260811000000_initial_greeting/migration.sqlprisma/migrations/20260812121700_initial_chat/migration.sqlprisma/migrations/migration_lock.tomlprisma/schema.prismaprisma/seed.tsscripts/local-database.tsscripts/setup.tsscripts/turso-database.tsscripts/turso-health.tsscripts/turso-migrate.tssrc/app/_components/attachment-tray.tsxsrc/app/_components/chat-view.tsxsrc/app/_components/composer.tsxsrc/app/_components/conversation-list.tsxsrc/app/_components/error-banner.tsxsrc/app/_components/model-picker.tsxsrc/app/_components/reasoning-disclosure.tsxsrc/app/_components/transcript.tsxsrc/app/_components/turn.tsxsrc/app/api/attachments/[attachmentId]/route.tssrc/app/api/chat/route.tssrc/app/api/completions/[messageId]/route.tssrc/app/api/conversations/[conversationId]/route.tssrc/app/api/models/route.tssrc/app/conversations/[conversationId]/page.tsxsrc/app/layout.tsxsrc/app/page.tsxsrc/features/attachment/read.tssrc/features/completion/history.tssrc/features/completion/messages.test.tssrc/features/completion/messages.tssrc/features/completion/parse.test.tssrc/features/completion/parse.tssrc/features/completion/run.tssrc/features/conversation/delete.tssrc/features/conversation/list.tssrc/features/conversation/read.tssrc/features/conversation/resolve.tssrc/features/conversation/title.test.tssrc/features/conversation/title.tssrc/features/greeting/read.tssrc/features/message/append.tssrc/features/message/close.tssrc/features/message/next-sequence.tssrc/features/message/start.tssrc/lib/abort-error.tssrc/lib/chat-event.test.tssrc/lib/chat-event.tssrc/lib/completion-connection.tssrc/lib/data-url.test.tssrc/lib/data-url.tssrc/lib/environment.test.tssrc/lib/environment.tssrc/lib/image-downscale.tssrc/lib/inference/client.tssrc/lib/inference/failure.test.tssrc/lib/inference/failure.tssrc/lib/inference/request.test.tssrc/lib/inference/request.tssrc/lib/inference/schema.tssrc/lib/inference/stream.test.tssrc/lib/inference/stream.tssrc/lib/prisma-client.tssrc/lib/running-completions.tssrc/lib/server-sent-events.test.tssrc/lib/server-sent-events.tstests/integration/attachment-route.test.tstests/integration/chat-route.test.tstests/integration/conversation-database.test.tstests/integration/fixtures/completion-chunks.tstests/integration/fixtures/conversation.tstests/integration/fixtures/database.tstests/integration/fixtures/inference-stub.tstests/integration/greeting-database.test.tstests/integration/models-route.test.tstests/system/attachment.spec.tstests/system/chat-page.tstests/system/chat.spec.tstests/system/failure.spec.tstests/system/fixtures/inference-server.tstests/system/home.spec.tstests/system/ports.tstests/system/reasoning.spec.tstests/system/stop.spec.ts
💤 Files with no reviewable changes (10)
- prisma/migrations/20260811000000_initial_greeting/migration.sql
- scripts/turso-migrate.ts
- prisma/seed.ts
- scripts/turso-health.ts
- src/features/greeting/read.ts
- tests/system/home.spec.ts
- scripts/turso-database.ts
- scripts/setup.ts
- tests/integration/greeting-database.test.ts
- .github/workflows/run-system-tests.yml
| echo '```text' | ||
| } >> "$GITHUB_STEP_SUMMARY" | ||
| bunx --bun react-doctor@latest . --project next-bun --verbose --offline --blocking none | tee -a "$GITHUB_STEP_SUMMARY" | ||
| bunx --bun react-doctor@latest . --no-score | tee -a "$GITHUB_STEP_SUMMARY" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 2 'react-doctor' .github/workflows/run-react-doctor.yml package.json bun.lock bun.lockb 2>/dev/null || trueRepository: akitorahayashi/vlm-chat
Length of output: 817
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- tracked manifests and lockfiles ---'
git ls-files | grep -E '(^|/)(package\.json|bun\.lockb?|npm-shrinkwrap\.json|package-lock\.json|yarn\.lock|pnpm-lock\.yaml)$' || true
printf '%s\n' '--- workflow ---'
cat -n .github/workflows/run-react-doctor.yml
printf '%s\n' '--- package manifests ---'
while IFS= read -r f; do
printf '\n--- %s ---\n' "$f"
cat "$f"
done < <(git ls-files | grep -E '(^|/)package\.json$' || true)Repository: akitorahayashi/vlm-chat
Length of output: 1362
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- package.json ---'
cat -n package.json
printf '%s\n' '--- react-doctor entries in bun.lock ---'
rg -n -C 4 'react-doctor' bun.lock || true
printf '%s\n' '--- setup action ---'
cat -n .github/actions/setup/action.ymlRepository: akitorahayashi/vlm-chat
Length of output: 3110
🌐 Web query:
react-doctor npm package documentation binary usage exact version
💡 Result:
The react-doctor npm package is a command-line tool designed to scan React codebases for issues related to state, effects, performance, architecture, security, and accessibility [1][2]. Binary Usage The recommended way to execute react-doctor is via npx, which ensures you are running the latest version [1][3]: npx react-doctor@latest [directory] [options] If the directory is omitted, the tool scans the current directory [4][5]. Common CLI commands include [4][5]: - General Audit: npx react-doctor@latest - Agent Skill Installation: npx react-doctor@latest install - CI Integration: npx react-doctor@latest ci install Basic options include [6][7]: - --verbose: Shows detailed file and rule information. - --json: Outputs a structured JSON report. - --no-lint: Skips linting diagnostics. - --no-dead-code: Skips dead code detection. - --fix: Opens Ami to automatically fix detected issues. Exact Version To run an exact version of react-doctor, you can specify the version string in the npx command instead of using @latest [1][7]. For example, to run version 0.9.8: npx react-doctor@0.9.8 Official documentation and further usage details are available at https://www.react.doctor/docs [2][8].
Citations:
- 1: https://www.npmjs.com/package/react-doctor
- 2: https://www.react.doctor/docs
- 3: https://www.react.doctor/docs/getting-started/run-your-first-scan.md
- 4: https://www.react.doctor/docs/reference/cli-reference
- 5: https://www.react.doctor/docs/reference/cli-reference.md
- 6: https://millionco-react-doctor-36.mintlify.app/usage/cli-commands
- 7: https://github.com/millionco/react-doctor/blob/db319d01b3d9ee0748545790afb03698e8d94997/README.md
- 8: https://react.doctor/docs
Pin the React Doctor executable.
react-doctor is absent from package.json and bun.lock. Add an exact dev dependency and invoke the project-installed binary without @latest, so CI uses the locked version.
🤖 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 @.github/workflows/run-react-doctor.yml at line 33, Add react-doctor as an
exact dev dependency so it is recorded in package.json and bun.lock, then update
the workflow command to invoke the project-installed binary without the `@latest`
suffix. Preserve the existing arguments and GitHub summary output behavior.
| ask model prompt="Reply with exactly: pong": | ||
| @curl -sf {{endpoint}}/v1/chat/completions \ | ||
| -H 'Content-Type: application/json' \ | ||
| -d '{"model":"{{model}}","messages":[{"role":"user","content":"{{prompt}}"}],"max_tokens":128}' \ | ||
| | jq -r '.choices[0].message | (.reasoning_content // empty), (.content // empty)' | ||
|
|
||
| # Download a model, after which it appears in `models`. | ||
| download model: | ||
| uv run hf download {{model}} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Serialize and shell-quote recipe parameters.
prompt and model are inserted into shell and JSON syntax without escaping. A prompt containing an apostrophe, quote, or newline can break ask. The same raw model interpolation affects download.
Build the request body with jq --arg or an equivalent JSON serializer. Shell-quote each recipe parameter before passing it to a command.
🤖 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 `@inference/justfile` around lines 48 - 56, Update the ask recipe to construct
its JSON request body with jq --arg or an equivalent serializer so prompt and
model values safely handle quotes, apostrophes, and newlines. Shell-quote every
interpolated recipe parameter in both ask and download, including endpoint,
model, and prompt, while preserving the existing request and download behavior.
| updatedAt DateTime @updatedAt | ||
| messages Message[] | ||
|
|
||
| @@index([updatedAt]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/features/message --items all --type function
rg -n -C 6 \
'prisma\.conversation\.(update|updateMany|upsert)|conversationId|updatedAt' \
src/features/messageRepository: akitorahayashi/vlm-chat
Length of output: 4758
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- message lifecycle files ---'
for file in src/features/message/start.ts src/features/message/close.ts src/features/message/append.ts; do
if [ -f "$file" ]; then
printf '\n--- %s ---\n' "$file"
cat -n "$file"
fi
done
printf '%s\n' '--- conversation listing and lifecycle call sites ---'
rg -n -C 8 \
'listConversations|closeAssistantMessage|startAssistantMessage|appendUserMessage|message\.(update|updateMany|create)|conversation\.(update|updateMany|upsert)' \
src
printf '%s\n' '--- schema models ---'
cat -n prisma/schema.prisma | sed -n '1,100p'Repository: akitorahayashi/vlm-chat
Length of output: 20339
Update Conversation for assistant activity.
startAssistantMessage and closeAssistantMessage do not update Conversation.updatedAt, so assistant activity can leave listConversations in the wrong order. Update the parent conversation in the same transaction as message creation and completion.
🤖 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 `@prisma/schema.prisma` around lines 17 - 20, Update the transaction paths used
by startAssistantMessage and closeAssistantMessage to also touch the parent
Conversation.updatedAt when creating or completing an assistant message. Ensure
each parent update occurs in the same transaction as its corresponding message
operation so listConversations ordering reflects assistant activity.
| } finally { | ||
| if (!settled) { | ||
| await close('aborted'); | ||
| } | ||
|
|
||
| releaseCompletion(input.assistantMessageId); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Completion cleanup depends on generator execution and can be skipped entirely. POST /api/chat calls registerCompletion before the completion generator starts, but the only release and the only fallback settlement live in the generator finally. A generator suspended before its first next() completes on return() without executing the body, and createServerSentEventStream calls iterator.return() from cancel. A client that abandons the response before the first chunk therefore leaves the assistant message in 'streaming' and leaves the AbortController in the registry for the process lifetime.
src/features/completion/run.ts#L87-L93: do not rely on thefinallyas the only cleanup site. Accept an explicit cleanup callback, or expose a settle-and-release function that the route can also invoke.src/lib/running-completions.ts#L10-L17: make the registry self-cleaning. Record the registration time, and remove or abort entries that were never released after a bounded interval, so an abandoned stream cannot retain a controller forever.
📍 Affects 2 files
src/features/completion/run.ts#L87-L93(this comment)src/lib/running-completions.ts#L10-L17
🤖 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/features/completion/run.ts` around lines 87 - 93, Ensure completion
cleanup does not depend solely on the generator finally block: in
src/features/completion/run.ts lines 87-93, expose or accept a
settle-and-release cleanup callback that the route can invoke when the stream is
abandoned before iteration begins, while preserving the existing generator
cleanup. In src/lib/running-completions.ts lines 10-17, make the completion
registry self-cleaning by recording registration time and removing or aborting
unreleased entries after a bounded timeout.
| export function isAbortError(error: unknown) { | ||
| return ( | ||
| error instanceof Error && | ||
| (error.name === 'AbortError' || error.name === 'TimeoutError') | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files \
'src/lib/abort-error.ts' \
'src/features/completion/run.ts' \
'src/app/api/chat/route.ts' \
'*package.json' \
'*lock*'
printf '%s\n' '--- abort-error.ts ---'
cat -n src/lib/abort-error.ts
printf '%s\n' '--- run.ts relevant symbols ---'
rg -n -C 12 'isAbortError|status:|inference|failed|failure|AbortError|TimeoutError' src/features/completion/run.ts
printf '%s\n' '--- chat route relevant symbols ---'
rg -n -C 12 'isAbortError|502|499|status|AbortError|TimeoutError|failed|inference' src/app/api/chat/route.ts
printf '%s\n' '--- abort helper usages ---'
rg -n -C 4 'isAbortError' srcRepository: akitorahayashi/vlm-chat
Length of output: 9413
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- package runtime targets and dependencies ---'
cat package.json
printf '%s\n' '--- relevant lockfile entries ---'
rg -n -C 3 'node|undici|next|typescript|`@types/node`' bun.lock mise.lock
printf '%s\n' '--- inference client ---'
cat -n src/lib/inference/client.ts
printf '%s\n' '--- stream decoder ---'
cat -n src/lib/inference/stream.ts
printf '%s\n' '--- tests and abort-related fixtures ---'
rg -n -i -C 3 'abort|timeout|terminated|ABORT_ERR|isAbortError' \
--glob '!bun.lock' --glob '!inference/uv.lock' --glob '!mise.lock' .Repository: akitorahayashi/vlm-chat
Length of output: 38851
🌐 Web query:
Node.js undici fetch response body streaming abort TypeError terminated cause AbortError
💡 Result:
When encountering errors related to aborting fetch response body streams in Undici (the engine behind Node.js fetch), such as TypeError: terminated or AbortError, it is important to understand that these often arise from how streams are handled when a request is cancelled while data is still being consumed [1][2][3][4]. Key Technical Context and Causes: 1. TypeError: terminated: This error typically occurs when the internal stream state is abruptly closed or terminated, often triggered during abnormal network conditions or when a response body is accessed after an abort has already occurred [3][5]. It indicates a low-level failure where the underlying connection was destroyed, and the consumer attempted to read from or interact with a stream that is no longer valid [3][5]. 2. AbortError and Expected Behavior: When using an AbortController to signal a fetch, the operation should ideally reject with a DOMException (AbortError) [1][2][6]. However, inconsistency exists [7]. If an abort occurs while reading a response body, issues can arise if the stream is not correctly handled or if it is already "locked" or "used" (e.g., after cloning) [2][8]. 3. Uncaught Exceptions and Process Crashes: A known issue exists where aborting a response stream can sometimes trigger an unhandled 'error' event on the ReadableStream [4][9]. This can cause the entire Node.js process to crash if the error is not explicitly caught or if the stream is not properly destroyed [5][4]. Recommendations for Handling: * Ensure Proper Error Handling: Always wrap fetch calls and subsequent body consumption in try/catch blocks [4][9]. Because some abort scenarios may emit an 'error' event on the stream itself, you may also need to listen for error events on the stream instance or use utilities like stream/promises 'finished' to ensure completion or failure are handled cleanly [4][10]. * Verify Response State: Before interacting with a response body, check if the request has been aborted via the signal [8]. Be aware that cloning a response can sometimes alter the aborting behavior, leading to TypeErrors (e.g., Body is unusable) instead of the expected AbortError [2]. * Avoid Unnecessary Stream Interaction: If you do not need the response body, avoid keeping the connection open unnecessarily [11]. For error responses, consuming the body or using specific methods to discard it (e.g., body.dump if available) is often cleaner than abrupt cancellation [11]. * Update Dependencies: Many of these issues are actively tracked and addressed in the Node.js and Undici repositories [1][2][5][10]. Ensure you are using the latest stable version of Node.js, which bundles the most recent version of Undici [5]. If you encounter a reproducible crash, check the official Node.js issue tracker for the latest status on relevant bugs [4][10].
Citations:
- 1: Awaiting response.json() hangs on an aborted request. nodejs/undici#4232
- 2: Cloning Response change default aborting behavior nodejs/undici#4799
- 3: Uncaught TypeError: terminated nodejs/undici#1688
- 4: Aborting a fetch response stream throws uncatchable exception nodejs/node#57360
- 5: fetch is sometimes crashing entire node process since upgrading from node 22.9.0 to 23.1.0 nodejs/undici#3813
- 6: https://github.com/node-fetch/node-fetch/blob/main/docs/ERROR-HANDLING.md
- 7: AbortSignal.timeout inconsistently leads to TimeoutError or AbortError nodejs/undici#2171
- 8: Response
bodyiteration doesn't throw if aborted before beginning iteration nodejs/undici#1940 - 9: Unhandled 'error' event when using AbortSignal to cancel requests with a body node-fetch/node-fetch#1420
- 10: fetch() doesn't cancel ReadableStream request body when connection closes (ECONNRESET) nodejs/node#61460
- 11: Is it better to consume not needed body or abort the request? nodejs/undici#2194
🌐 Web query:
Node.js abortable APIs ABORT_ERR error code AbortError documentation
💡 Result:
In Node.js, ABORT_ERR is an error code used when an operation is aborted, typically via an AbortController or AbortSignal [1][2]. It was introduced in Node.js v15.0.0 [1][3]. Key characteristics of ABORT_ERR and AbortError include: 1. Compatibility with Web Standards: Unlike most Node.js errors that follow the ERR_* naming convention, ABORT_ERR was specifically chosen to remain compatible with the web platform's AbortError [1][4]. 2. Error Structure: When a Node.js-specific API (such as those in events, streams, or timers) is aborted using an AbortSignal, the resulting error object typically has its.code property set to ABORT_ERR and its.name property set to AbortError [5]. 3. DOMException vs. Native Errors: - Web-standard APIs (e.g., fetch, Web Streams) generally use the DOMException class for abort errors to ensure cross-runtime compatibility [5]. - Many Node.js-specific internal APIs use a native Error object with the code ABORT_ERR [5]. - Developers are encouraged to check for abort conditions by verifying if an error's.name property is 'AbortError' (i.e., if (err?.name === 'AbortError')), as this approach works across both web browsers and Node.js [5][6]. 4. Propagation of Reasons: Modern Node.js APIs support passing a reason to abortController.abort(reason). If provided, this reason is often attached to the AbortSignal and can be accessed via the signal.reason property, or it may be propagated as the cause of the resulting AbortError [7][8]. For more information, see the official Node.js documentation on Error codes [1] and Global objects [7].
Citations:
- 1: https://nodejs.org/api/errors.html
- 2: https://nodejs.org/docs/latest-v23.x/api/errors.html
- 3: https://nodejs.org/docs/v22.11.0/api/errors.html
- 4: https://nodejs.org/docs/latest/api/errors.html
- 5: doc:
DOMExceptionmissing from “Errors” API reference docs page nodejs/node#40789 - 6: Thrown AbortErrors are not DOMExceptions nodejs/node#40692
- 7: https://nodejs.org/api/globals.html
- 8: lib: propagate AbortSignal.reason as AbortError
causewhere appropriate nodejs/node#41008
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- available standalone runtimes ---'
command -v bun || true
command -v node || true
bun --version 2>/dev/null || true
node --version 2>/dev/null || true
printf '%s\n' '--- integration abort fixtures and stream reader ---'
sed -n '1,290p' tests/integration/chat-route.test.ts
printf '%s\n' '--- server-sent-events reader ---'
cat -n src/lib/server-sent-events.ts
printf '%s\n' '--- runtime/config references ---'
rg -n -i -C 4 'bun|node|runtime|undici|fetch' \
README.md CONTRIBUTING.md docs src tests package.json mise.toml mise.lock 2>/dev/null || trueRepository: akitorahayashi/vlm-chat
Length of output: 37216
🌐 Web query:
Bun 1.3.14 fetch abort response body stream TypeError terminated cause AbortError
💡 Result:
The TypeError: terminated error occurring in Bun (and similarly in Node.js/undici) when fetching with an AbortController is a standard, expected mechanism indicating that a fetch operation or its response body stream was forcefully closed due to an abort signal [1][2][3]. In technical terms, when a fetch operation is aborted, the underlying HTTP connection is terminated [1][4]. If a user attempts to continue reading from the response body stream after this termination has occurred, the stream will throw a TypeError with the message terminated, often carrying an AbortError DOMException as the cause [1][2][3]. Key points regarding this behavior: 1. Nature of the Error: It is not a bug in your code, but rather a reflection of the stream being closed [3]. When an AbortSignal is triggered, the Fetch implementation ensures that any active processing—such as network I/O or pending stream reads—is halted [5][6][4]. 2. Handling the Error: Because this is a normal lifecycle event for an aborted request, it is best practice to handle it gracefully [3]. You can catch this error and check the error type or name to determine if it is an expected abort [3]: try { const response = await fetch(url, { signal }); const data = await response.json; } catch (err) { if (err.name === 'AbortError' || (err instanceof TypeError && err.message === 'terminated')) { // This is an expected abort; handle it gracefully return; } throw err; // Handle actual network or processing errors } 3. Recent Context in Bun: Bun has undergone significant work to align its stream teardown and abort behavior with Fetch and Node.js standards [6][4]. This includes ensuring that aborting a fetch properly errors the response body stream [6], and that cancelling a stream reader (e.g., via reader.cancel) properly propagates the abort to the underlying connection [4][7]. If you are encountering this error unexpectedly, ensure that your AbortController is not being triggered prematurely (e.g., due to a racing navigation or rapid retry logic) [8][9], and verify that you are not attempting to access the body stream after the operation has been cancelled [10].
Citations:
- 1:
undici.fetchfails (terminates) on 3XX withredirect: "manual"nodejs/undici#1622 - 2: https://stackoverflow.com/questions/74630834/node-js-fetch-response-json-throws-typeerror
- 3: Client error for command SSE stream disconnected: TypeError: terminated modelcontextprotocol/typescript-sdk#949
- 4: fetch: abort the request when the response body reader is cancelled oven-sh/bun#33231
- 5: Aborting fetch whatwg/fetch#523
- 6: fetch: error the response body stream when a fully-buffered response is aborted oven-sh/bun#35093
- 7: fix: abort HTTP connection when response body stream is cancelled oven-sh/bun#27232
- 8: fix(router): suppress abort errors in single fetch remix-run/react-router#14761
- 9: https://sujeet.pro/articles/fetch-and-streaming-apis
- 10: [Bug] fetch() violates JavaScript semantics: Response body destroyed after preemptive consumption, breaking cross-function stream passing oven-sh/bun#28952
🌐 Web query:
Bun fetch AbortController abort error name code cause response.body reader
💡 Result:
In Bun, the behavior of AbortController and response.body reader cancellation has undergone significant refinement to align with web standards and resolve resource leaks [1][2][3][4]. Error Name and Code When a fetch request is aborted using an AbortController, the resulting error is typically an AbortError with the message "The operation was aborted" [5]. Historically, some Bun versions reported this as a generic "Error" [5], but current implementations support standard AbortError checks [6]. If a custom reason is provided to controller.abort(reason), that reason is surfaced [1]. For general network failures (which are distinct from aborts), Bun throws a TypeError with the message "fetch failed", which includes a .cause property containing the underlying system error [7]. Response Body Reader Behavior 1. AbortController: When you abort a fetch request, it should signal the underlying stream to error [1]. This causes any pending reads on the response.body reader to reject with the abort reason [1]. 2. Reader Cancellation: Calling reader.cancel() (or response.body.cancel()) on a fetch response is designed to close the underlying HTTP connection [2][3]. This propagates the cancellation to the server, ensuring that resources are freed and the server is notified that the client is no longer consuming the stream [2][3][4]. This behavior applies even when breaking from for await loops over the response body [4]. Resource Management There have been known issues where improper handling of aborted streams led to memory leaks (retaining response bodies off-heap) [8]. Current Bun updates have focused on ensuring that when a stream is aborted or cancelled, the native backing stores are correctly released [8][1][3]. It is recommended to use reader.cancel() for explicitly ending consumption, as this is reliably linked to connection teardown [2][3]. Summary of Key Behaviors - AbortError: The standard error for aborted requests [6]. - Propagated Cancellation: Cancelling the reader now triggers an abort of the HTTP connection, preventing the server from continuing to send data [2][3]. - Error Transparency: Fetch failures now include detailed .cause information and proper stack traces [7]. If you encounter unexpected errors, ensure you are on a recent version of Bun, as significant fixes for stream lifecycle management and abort signal propagation were integrated in releases following early implementation phases [1][3][4].
Citations:
- 1: fetch: error the response body stream when a fully-buffered response is aborted oven-sh/bun#35093
- 2: fetch: abort the request when the response body reader is cancelled oven-sh/bun#33231
- 3: fix: abort HTTP connection when fetch response reader is cancelled oven-sh/bun#27258
- 4: fix: abort HTTP connection when response body stream is cancelled oven-sh/bun#27232
- 5: Catch doesn't works as expected with AbortController oven-sh/bun#9805
- 6: oven-sh/bun@9657f37896cd
- 7: fetch: reject network errors as TypeError('fetch failed') with cause, errno codes, and a caller stack oven-sh/bun#35998
- 8: fetch(): aborting an in-flight streaming response via AbortController retains the response body off-heap — RSS grows unbounded until OOM (HTTP/1.1; reader.cancel() does not) oven-sh/bun#32659
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- standalone shape verifier ---'
node - <<'JS'
function current(error) {
return (
error instanceof Error &&
(error.name === 'AbortError' || error.name === 'TimeoutError')
);
}
const ABORT_NAMES = new Set(['AbortError', 'TimeoutError']);
function proposed(error, depth = 0) {
if (depth > 3 || typeof error !== 'object' || error === null) {
return false;
}
const candidate = error;
if (typeof candidate.name === 'string' && ABORT_NAMES.has(candidate.name)) {
return true;
}
if (candidate.code === 'ABORT_ERR') {
return true;
}
return proposed(candidate.cause, depth + 1);
}
const domAbort = Object.assign(new Error('The operation was aborted.'), {
name: 'AbortError',
});
const wrappedUndici = new TypeError('terminated', { cause: domAbort });
const nodeAbort = Object.assign(new Error('The operation was aborted.'), {
code: 'ABORT_ERR',
});
const deepWrapped = { cause: { cause: { cause: { cause: domAbort } } } };
for (const [label, value] of Object.entries({
domAbort,
wrappedUndici,
nodeAbort,
deepWrapped,
ordinary: new TypeError('terminated'),
})) {
console.log(label, JSON.stringify({
current: current(value),
proposed: proposed(value),
}));
}
JS
printf '%s\n' '--- Bun upstream references for abort stream behavior ---'
for url in \
'https://api.github.com/repos/oven-sh/bun/pulls/35093' \
'https://api.github.com/repos/oven-sh/bun/pulls/33231' \
'https://api.github.com/repos/oven-sh/bun/issues/28952'; do
printf '%s\n' "$url"
curl -fsSL -H 'Accept: application/vnd.github+json' "$url" |
jq '{number, state, title, body, merged_at, merge_commit_sha}' |
sed -n '1,100p'
doneRepository: akitorahayashi/vlm-chat
Length of output: 11712
🌐 Web query:
site:github.com/oven-sh/bun "TypeError: terminated" AbortError cause
💡 Result:
The error message "TypeError: terminated" appearing with an "AbortError" cause is characteristic of fetch network operations in the Bun runtime [1][2]. This specific error structure typically occurs when a network request (such as fetch) fails or is interrupted after the response headers have already been received [1][2]. Recent updates to Bun have aligned its error handling more closely with the Fetch specification and other runtimes (like Node.js/undici) by rejecting network failures with a TypeError that includes a.cause property, providing diagnostic information about the underlying system error (e.g., connection reset, DNS failure) [1][3][2]. Key technical aspects include: 1. Error Shape: Bun now materializes fetch network errors as a TypeError with the message "fetch failed" or "terminated" (the latter specifically when response headers have already arrived) [1][2]. 2. Diagnostic Information: The.cause property contains the original system error object, which preserves details like the error code (e.g., ECONNRESET), syscall, and hostname, ensuring diagnostic data is not lost [1][3][4]. 3. Context: While "AbortError" is often associated with the use of AbortSignal, the "TypeError: terminated" message is distinct and specifically indicates a failure during the streaming or processing phase of an established connection [1][2]. If you are encountering this, it is likely due to an issue with the network stream being interrupted or a server-side disconnect after the request started [1][2]. You can typically inspect the.cause property of the thrown error to identify the specific network-level failure code [1][2][4]. If you are experiencing "uncatchable" AbortErrors or termination crashes in specific libraries, ensure you are running an up-to-date version of the runtime, as many reported issues regarding abort signal handling and process termination have been addressed in recent Bun releases [5][6][7].
Citations:
- 1: fetch: reject network errors as TypeError('fetch failed') with cause, errno codes, and a caller stack oven-sh/bun#35998
- 2: fetch: mark body disturbed when a reader fails; reject network errors as TypeError oven-sh/bun#35855
- 3: fetch: reject network errors as TypeError('fetch failed' / 'terminated') with the system error as cause oven-sh/bun#35988
- 4: fetch: attach a node-style cause to connection-failure errors oven-sh/bun#34402
- 5: http.request: emit 'error' with AbortError when options.signal aborts oven-sh/bun#31170
- 6: AbortSignal.timeout() is not respected for http.request oven-sh/bun#31167
- 7: AbortController usage in module scope crashes bun with uncatchable error oven-sh/bun#9429
🌐 Web query:
site:github.com/nodejs/undici "TypeError: terminated" "cause" "AbortError"
💡 Result:
The error TypeError: terminated is a generic error message primarily encountered in Node.js when using the built-in fetch API (powered by the undici library) [1][2][3]. It indicates that an HTTP request or stream operation has been abruptly closed or aborted before completion [2][4][5]. Because the error is generic, it acts as a wrapper for various underlying issues [1]. The true cause is typically found in the error's cause property [4][3]. Common causes include: 1. Connection Issues: Underlying network problems like ECONNRESET (a connection reset by the peer) or other socket-level interruptions often trigger this error [3]. 2. Manual Aborts: If you are using an AbortController, the operation may be intentionally aborted [4]. 3. Stream Handling Errors: The error often occurs during stream consumption, such as when attempting to read a response body that has been closed due to a redirect, a server-side timeout, or protocol-level mismatch (e.g., trying to read a body from a response status that does not support one) [2][4][6]. 4. Compatibility Issues: In some cases, it can arise from version mismatches between Node.js and the underlying undici implementation, where certain features (like specific web stream handling) may not be fully supported in older Node.js versions [7]. To debug this error: 1. Inspect the cause property: Log the error object in your catch block to view the cause. For example: console.error(err.cause) [1][4][3]. 2. Check for network interruptions: If the cause is ECONNRESET or similar, the issue likely resides with the server or a middleman (proxy/firewall) dropping the connection [3]. 3. Validate Request Parameters: Ensure you are not misusing options like { redirect: 'manual' }, which can cause streams to hang or terminate prematurely [4][6]. 4. Update Node.js: Ensure you are using a supported and up-to-date version of Node.js, as many related bugs have been addressed in newer releases of Node.js and the undici library [3][7].
Citations:
- 1: [Regression] #fetch() is now returning generic
terminatederror message in some cases nodejs/undici#1140 - 2: Uncaught TypeError: terminated nodejs/undici#1688
- 3: fetch is sometimes crashing entire node process since upgrading from node 22.9.0 to 23.1.0 nodejs/undici#3813
- 4:
undici.fetchfails (terminates) on 3XX withredirect: "manual"nodejs/undici#1622 - 5: Undici throws a terminated error nodejs/undici#1490
- 6: fix(fetch): hangs on a stream response with manual redirect nodejs/undici#1627
- 7:
undici.fetchbreaks on Node.js <=18.13.0 in undici@6.13.0 nodejs/undici#3123
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Bun release and fix timing ---'
for url in \
'https://api.github.com/repos/oven-sh/bun/releases/tags/bun-v1.3.14' \
'https://api.github.com/repos/oven-sh/bun/commits/789be97db9b746533cf692e8367146e2d3c0d7cb' \
'https://api.github.com/repos/oven-sh/bun/pulls/35998' \
'https://api.github.com/repos/oven-sh/bun/pulls/35855'; do
printf '%s\n' "$url"
curl -fsSL -H 'Accept: application/vnd.github+json' "$url" |
jq '{tag_name, name, published_at, number, title, merged_at, merge_commit_sha, body}' |
sed -n '1,120p'
doneRepository: akitorahayashi/vlm-chat
Length of output: 9321
🌐 Web query:
Bun v1.3.14 release date fetch abort stream
💡 Result:
Bun v1.3.14 was released on May 13, 2026 [1][2]. Regarding your query about fetch, abort, and streams, Bun v1.3.14 included a specific fix to improve the integration between ReadableStream cancellation and HTTP connection management [3]. Previously, if you broke from a for await loop over a response.body stream, the stream cancellation did not correctly propagate to the underlying HTTP client, causing the connection to remain open until the full response was received [3]. In this release, Bun implemented a mechanism to properly abort the underlying HTTP connection when the stream is cancelled, allowing the process to exit or release resources immediately [3]. This was achieved by: - Adding cancel_handler and cancel_ctx fields to ReadableStream.NewSource to invoke a callback when the stream is cancelled [3]. - Registering a cancel handler in FetchTasklet that calls abortTask() to schedule the shutdown of the HTTP socket [3]. Please note that users have reported issues following this release, such as potential segmentation faults when using Response.body / ReadableStream finalizers in certain proxy scenarios (e.g., SSE streams), which may be related to the underlying handling of these stream objects [4].
Citations:
- 1: https://github.com/oven-sh/bun/releases/tag/bun-v1.3.14
- 2: https://bun.sh/blog/bun-v1.3.14
- 3: fix: abort HTTP connection when response body stream is cancelled oven-sh/bun#27232
- 4: Bun 1.3.14 segfault in Response.body / ReadableStream finalizer on Linux x64 oven-sh/bun#31159
🌐 Web query:
Bun 1.3.14 release notes AbortError fetch response body stream
💡 Result:
Bun 1.3.14, released on May 13, 2026, includes improvements and fixes related to the fetch API [1][2], specifically addressing how response body streams handle AbortError [3]. While the Bun 1.3.14 release notes highlight major features such as Bun.Image [1][4], HTTP/3 support [1], and experimental HTTP/2 and HTTP/3 clients [1], they also encompass various bug fixes for Web APIs [1][4]. A key improvement relevant to your query is a change that aligns Bun's fetch() behavior with the Fetch specification and Node.js regarding AbortController signals [3]. Specifically, changes integrated around this timeframe ensure that when a fetch() operation is aborted: 1. The response body stream is correctly errored (rejecting pending reads with AbortError or the custom abort reason) [3]. 2. The backing native store is released, preventing memory leaks that occurred in previous versions when aborting streaming responses [3][5]. 3. Bun now consistently rejects arrayBuffer(), text(), and streaming reads with AbortError if the fetch is aborted, matching the behavior of other environments like Node.js [3]. Before this change, aborting a fully-buffered response could sometimes result in the response resolving normally, or aborting a streaming response could leave the body cached off-heap, potentially leading to increased memory usage (RSS growth) [3][5]. Bun 1.3.14 includes these updates to ensure proper stream lifecycle management and spec-compliant error handling [3].
Citations:
- 1: https://bun.com/blog/bun-v1.3.14
- 2: https://github.com/oven-sh/bun/releases/tag/bun-v1.3.14
- 3: fetch: error the response body stream when a fully-buffered response is aborted oven-sh/bun#35093
- 4: https://bun.sh/blog/bun-v1.3.14
- 5: fetch(): aborting an in-flight streaming response via AbortController retains the response body off-heap — RSS grows unbounded until OOM (HTTP/1.1; reader.cancel() does not) oven-sh/bun#32659
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Bun v1.3.14 tag object and commit ---'
curl -fsSL -H 'Accept: application/vnd.github+json' \
'https://api.github.com/repos/oven-sh/bun/git/ref/tags/bun-v1.3.14' |
jq '{ref, object}'
printf '%s\n' '--- related merged pull requests ---'
for n in 35093 33231 35855; do
curl -fsSL -H 'Accept: application/vnd.github+json' \
"https://api.github.com/repos/oven-sh/bun/pulls/$n" |
jq '{number,title,state,merged_at,merge_commit_sha}'
done
printf '%s\n' '--- whether post-release fixes are in v1.3.14 ---'
tag_sha=$(curl -fsSL -H 'Accept: application/vnd.github+json' \
'https://api.github.com/repos/oven-sh/bun/git/ref/tags/bun-v1.3.14' |
jq -r '.object.sha')
for sha in \
789be97db9b746533cf692e8367146e2d3c0d7cb \
85300d25bf8d5e1b1a35c5b74b669b546adefe5d \
8afcd4b45d3172972cd4fbead1d32680169f72ed; do
printf '%s\n' "$sha"
curl -fsSL -H 'Accept: application/vnd.github+json' \
"https://api.github.com/repos/oven-sh/bun/compare/$tag_sha...$sha" |
jq '{status,ahead_by,behind_by}'
doneRepository: akitorahayashi/vlm-chat
Length of output: 1555
🌐 Web query:
Bun v1.3.14 fetch response body abort TypeError terminated exact error shape
💡 Result:
In Bun v1.3.14, the TypeError: terminated error is a standard network error shape that indicates a fetch request was interrupted or failed after response headers were already received [1][2]. This error typically occurs when an underlying HTTP connection is closed or aborted while a response body is still being read [3][4]. Key characteristics and causes include: Error Shape The error is surfaced as a TypeError with the message "terminated" [1][2]. It is designed to be catchable and includes a.cause property (where applicable) containing underlying system information, such as ECONNRESET or other socket-level error codes, to help distinguish between different failure modes [1][2][5]. Common Triggers 1. Mid-stream Connection Loss: The most common cause is the server closing the TCP/TLS connection unexpectedly while the client is still consuming the ReadableStream (e.g., in a streaming chat or SSE response) [3][4]. 2. Abort Signal vs. Reader Cancellation: While newer Bun versions have improved how reader.cancel and AbortController.abort propagate to the network layer, discrepancies can still arise [6][7][8]. Using reader.cancel is generally the preferred way to stop consuming a body and trigger proper connection teardown, as aborting a request via AbortController mid-body in certain versions may not always immediately release resources [8]. 3. Unconsumed Bodies: If a Response body is not fully consumed or properly closed, the connection may remain in an unstable state, eventually leading to termination errors [4]. Development Context Bun has actively worked to unify its fetch error reporting to match standards (like Node/Undici), ensuring that network failures consistently reject as TypeErrors with appropriate diagnostic information (.code,.syscall,.cause) rather than plain Errors or process crashes [1][9][2][5]. If you are encountering this error consistently, ensure you are correctly consuming the response body or explicitly calling reader.cancel when you are finished, and check that the server is not prematurely closing the connection [3][4].
Citations:
- 1: fetch: reject network errors as TypeError('fetch failed') with cause, errno codes, and a caller stack oven-sh/bun#35998
- 2: fetch: mark body disturbed when a reader fails; reject network errors as TypeError oven-sh/bun#35855
- 3: TypeError: terminated — TLS socket closes mid-stream during SSE response OpenRouterTeam/ai-sdk-provider#412
- 4: https://stackoverflow.com/questions/76931498/typeerror-terminated-cause-socketerror-other-side-closed-in-fetch-nodejs
- 5: fetch: attach a node-style cause to connection-failure errors oven-sh/bun#34402
- 6: fetch: abort the request when the response body reader is cancelled oven-sh/bun#33231
- 7: fix: abort HTTP connection when fetch response reader is cancelled oven-sh/bun#27258
- 8: fetch(): aborting an in-flight streaming response via AbortController retains the response body off-heap — RSS grows unbounded until OOM (HTTP/1.1; reader.cancel() does not) oven-sh/bun#32659
- 9: http: reject fetch() instead of panicking when the HTTP client thread fails to spawn oven-sh/bun#32245
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Bun issue and pull request describing still-streaming aborts ---'
for endpoint in \
'https://api.github.com/repos/oven-sh/bun/issues/32662' \
'https://api.github.com/repos/oven-sh/bun/pulls/32662' \
'https://api.github.com/repos/oven-sh/bun/issues/32659'; do
printf '%s\n' "$endpoint"
curl -fsSL -H 'Accept: application/vnd.github+json' "$endpoint" |
jq '{number,title,state,created_at,closed_at,merged_at,merge_commit_sha,body}' |
sed -n '1,180p'
done
printf '%s\n' '--- existing cause-chain classifier style ---'
cat -n src/lib/inference/failure.tsRepository: akitorahayashi/vlm-chat
Length of output: 14997
Classify aborted streams before incomplete-stream failures.
Bun 1.3.14 can end an aborted streaming response with { done: true } instead of rejecting. decodeCompletionStream then emits an incomplete-stream error, and runCompletion stores status: 'failed'. Pass the relevant AbortSignal into stream handling and classify signal.aborted as aborted before reporting a stream failure. Retain cause-chain checks for runtimes that reject with wrapped AbortError or code: 'ABORT_ERR'. Add a Bun 1.3.14 regression 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/lib/abort-error.ts` around lines 1 - 6, Update stream handling around
decodeCompletionStream and runCompletion to accept the relevant AbortSignal,
classify signal.aborted as an aborted outcome before emitting incomplete-stream
failures, and preserve existing cause-chain detection for wrapped AbortError or
code ABORT_ERR rejections. Add a regression test covering Bun 1.3.14 behavior
where an aborted stream ends with done: true.
Prevent race conditions where a second completion request builds a prompt from a state missing the previous reply. Use a client-provided completionId to manage concurrency and ensure cancellations can be triggered before the assistant message row is created.
- Enforce base64 byte alignment to prevent partial decoding. - Improve error handling for conversation deletion and chat stream failures. - Add timeouts and strict schema validation for inference server responses. - Encapsulate inference stub lifecycles in tests to prevent cross-contamination. - Refine React Doctor CI job configuration for version stability.
- Force `--webpack` in `dev` and test server commands to avoid Turbopack module resolution errors with database dependencies. - Move running completions state to `globalThis` to persist across module reloads during `next dev`. - Update test configuration to run with a single worker to avoid race conditions in browser tests. - Switch test database initialization from `db:setup` to `db:reset`.
There was a problem hiding this comment.
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 `@src/app/api/chat/route.ts`:
- Around line 124-141: Update the catch block around startAssistantMessage to
preserve the existing generation.abort() and upstream.cancel() cleanup, then
return a structured 500 CompletionRejected response containing conversation.id
instead of rethrowing the error. Ensure the client receives the stored
conversation ID for retry handling.
In `@src/lib/inference/client.ts`:
- Around line 59-75: Update fetchModelIds so the /v1/models response body read,
including response.json(), occurs inside the existing try/catch that handles
request timeouts. Preserve the endpoint-specific timeout error when the deadline
aborts during either headers or body completion, and add a test covering a
response whose headers arrive but whose body stalls.
In `@src/lib/running-completions.ts`:
- Around line 52-65: Update claimCompletion to reject input.completionId values
already present in running, in addition to the existing byConversation check,
before writing either map. Make the corresponding release/cancellation logic
ownership-aware so a stale completion cannot remove or alter a newer claim;
preserve the original conversation’s claim until its generator finishes if
cancellation does not immediately release it.
In `@tests/integration/chat-route.test.ts`:
- Around line 362-366: Update the test around response.body.cancel() and
readMessages so it polls the assistant row until its status leaves streaming
before asserting aborted. Use a bounded wait to avoid hanging indefinitely, then
preserve the existing aborted-status assertion.
- Around line 277-298: Replace the fixed Bun.sleep(80) in the “stops a turn that
is still waiting for the model to load” test with a bounded poll that repeatedly
calls cancel(completionId) until it returns { cancelled: true }. Add a timeout
to fail deterministically if cancellation never succeeds, then assert the
successful cancellation response and preserve the pending request status
assertion.
In `@tests/integration/fixtures/inference-stub.ts`:
- Around line 104-113: Update the inference-stub cleanup wrapper around stop to
await stub.stop() before restoring VLM_CHAT_INFERENCE_URL, and make every
fixture cleanup call await stop() (including stopServer() before the closed-port
request) so shutdown completes before subsequent operations.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 501275ce-d790-43aa-9c7d-f04cbcc994a7
📒 Files selected for processing (33)
.env.example.github/workflows/run-react-doctor.ymlAGENTS.mdREADME.mdinference/justfilepackage.jsonplaywright.config.tssrc/app/_components/attachment-tray.tsxsrc/app/_components/chat-view.tsxsrc/app/_components/conversation-list.tsxsrc/app/_components/turn.tsxsrc/app/api/attachments/[attachmentId]/route.tssrc/app/api/chat/route.tssrc/app/api/completions/[completionId]/route.tssrc/app/api/conversations/[conversationId]/route.tssrc/features/completion/history.tssrc/features/completion/messages.test.tssrc/features/completion/messages.tssrc/features/completion/parse.test.tssrc/features/completion/parse.tssrc/features/completion/run.tssrc/features/message/close.tssrc/lib/completion-connection.tssrc/lib/image-downscale.tssrc/lib/inference/client.tssrc/lib/inference/schema.tssrc/lib/inference/stream.test.tssrc/lib/running-completions.tstests/integration/chat-route.test.tstests/integration/conversation-database.test.tstests/integration/fixtures/inference-stub.tstests/integration/models-route.test.tstests/system/failure.spec.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- .github/workflows/run-react-doctor.yml
- package.json
- src/app/_components/conversation-list.tsx
- src/features/completion/messages.ts
- src/app/_components/attachment-tray.tsx
- tests/integration/conversation-database.test.ts
- src/app/_components/chat-view.tsx
- src/features/message/close.ts
- src/features/completion/parse.test.ts
- src/lib/inference/stream.test.ts
- src/app/api/attachments/[attachmentId]/route.ts
- inference/justfile
- src/features/completion/messages.test.ts
- playwright.config.ts
- src/app/_components/turn.tsx
- README.md
- tests/integration/models-route.test.ts
| try { | ||
| assistantMessage = await startAssistantMessage({ | ||
| conversationId: conversation.id, | ||
| modelId: parsed.modelId, | ||
| seed, | ||
| }); | ||
| } catch (error) { | ||
| // The server is already generating and nothing downstream will ever read | ||
| // it, so it has to be told to stop here: no assistant turn exists to | ||
| // carry the tokens, and the model would stay busy to the end. | ||
| generation.abort(); | ||
|
|
||
| // Closes the body on a runtime where aborting a settled fetch does not. | ||
| // It rejects when the abort above already errored the stream, which is | ||
| // the expected case and says nothing a caller could act on. | ||
| void upstream.cancel().catch(() => {}); | ||
|
|
||
| throw error; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Return the stored conversation ID when assistant-message creation fails.
appendUserMessage has already stored the user turn when this catch block runs. Re-throwing produces an unstructured error response, so CompletionRejected cannot provide conversation.id to the client. A retry from the root page then creates a second conversation.
Return a structured 500 rejection with conversation.id after aborting and cancelling upstream.
Proposed fix
- throw error;
+ return reject(
+ 'Could not start the assistant reply.',
+ 500,
+ conversation.id,
+ );📝 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.
| try { | |
| assistantMessage = await startAssistantMessage({ | |
| conversationId: conversation.id, | |
| modelId: parsed.modelId, | |
| seed, | |
| }); | |
| } catch (error) { | |
| // The server is already generating and nothing downstream will ever read | |
| // it, so it has to be told to stop here: no assistant turn exists to | |
| // carry the tokens, and the model would stay busy to the end. | |
| generation.abort(); | |
| // Closes the body on a runtime where aborting a settled fetch does not. | |
| // It rejects when the abort above already errored the stream, which is | |
| // the expected case and says nothing a caller could act on. | |
| void upstream.cancel().catch(() => {}); | |
| throw error; | |
| try { | |
| assistantMessage = await startAssistantMessage({ | |
| conversationId: conversation.id, | |
| modelId: parsed.modelId, | |
| seed, | |
| }); | |
| } catch (error) { | |
| // The server is already generating and nothing downstream will ever read | |
| // it, so it has to be told to stop here: no assistant turn exists to | |
| // carry the tokens, and the model would stay busy to the end. | |
| generation.abort(); | |
| // Closes the body on a runtime where aborting a settled fetch does not. | |
| // It rejects when the abort above already errored the stream, which is | |
| // the expected case and says nothing a caller could act on. | |
| void upstream.cancel().catch(() => {}); | |
| return reject( | |
| 'Could not start the assistant reply.', | |
| 500, | |
| conversation.id, | |
| ); |
🤖 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/app/api/chat/route.ts` around lines 124 - 141, Update the catch block
around startAssistantMessage to preserve the existing generation.abort() and
upstream.cancel() cleanup, then return a structured 500 CompletionRejected
response containing conversation.id instead of rethrowing the error. Ensure the
client receives the stored conversation ID for retry handling.
| export async function fetchModelIds(signal?: AbortSignal) { | ||
| const deadline = AbortSignal.timeout(MODEL_LIST_TIMEOUT_MS); | ||
| let attempt: Awaited<ReturnType<typeof request>>; | ||
|
|
||
| try { | ||
| attempt = await request('/v1/models', { | ||
| signal: signal ? AbortSignal.any([signal, deadline]) : deadline, | ||
| }); | ||
| } catch (error) { | ||
| if (deadline.aborted && !signal?.aborted) { | ||
| throw new Error( | ||
| `The inference server at ${getInferenceEndpoint()} did not answer within ${MODEL_LIST_TIMEOUT_MS / 1000} seconds.`, | ||
| ); | ||
| } | ||
|
|
||
| throw error; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(client|inference|.*test.*|.*spec.*)' | head -200
printf '%s\n' '--- client outline ---'
ast-grep outline src/lib/inference/client.ts || true
printf '%s\n' '--- relevant symbols ---'
rg -n -C 8 'fetchModelIds|MODEL_LIST_TIMEOUT_MS|function request|const request|response\.json|AbortSignal' src/lib/inference src --glob '*.{ts,tsx,js,mjs,cjs}' | head -300Repository: akitorahayashi/vlm-chat
Length of output: 16906
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- client.ts ---'
cat -n src/lib/inference/client.ts
printf '%s\n' '--- request.test.ts ---'
cat -n src/lib/inference/request.test.ts
printf '%s\n' '--- inference test references ---'
rg -n -C 5 'fetchModelIds|fetch\(|AbortSignal|models' src/lib/inference src/app tests --glob '*.{ts,tsx}' | head -350
printf '%s\n' '--- package scripts/runtime hints ---'
rg -n '"(test|typecheck|lint|dev|build)"|bun:test|vitest|jest' package.json bun.lockb package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null | head -120 || true
printf '%s\n' '--- standalone delayed-body probe ---'
node - <<'JS'
const timeoutMs = 25;
const deadline = AbortSignal.timeout(timeoutMs);
const response = {
async json() {
await new Promise((resolve, reject) => {
const onAbort = () => reject(deadline.reason);
if (deadline.aborted) return onAbort();
deadline.addEventListener('abort', onAbort, { once: true });
});
return { data: [] };
},
};
async function fetchModelIdsLike() {
try {
await Promise.resolve({ response });
} catch (error) {
if (deadline.aborted) throw new Error('endpoint-specific timeout');
throw error;
}
return response.json();
}
try {
await fetchModelIdsLike();
} catch (error) {
console.log(JSON.stringify({
name: error.name,
message: error.message,
endpointSpecific: error.message === 'endpoint-specific timeout',
deadlineAborted: deadline.aborted,
}));
}
JSRepository: akitorahayashi/vlm-chat
Length of output: 23476
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- inference stub ---'
cat -n tests/integration/fixtures/inference-stub.ts
printf '%s\n' '--- models route tests ---'
cat -n tests/integration/models-route.test.ts
printf '%s\n' '--- corrected delayed-body probe ---'
node - <<'JS'
const timeoutMs = 25;
const deadline = AbortSignal.timeout(timeoutMs);
let catchRan = false;
const response = {
async json() {
await new Promise((resolve, reject) => {
const onAbort = () => reject(deadline.reason);
if (deadline.aborted) return onAbort();
deadline.addEventListener('abort', onAbort, { once: true });
});
return { data: [] };
},
};
async function fetchModelIdsLike() {
try {
await Promise.resolve({ response }); // headers arrive before the deadline
} catch (error) {
catchRan = true;
if (deadline.aborted) throw new Error('endpoint-specific timeout');
throw error;
}
return response.json(); // body read is outside the timeout catch
}
(async () => {
try {
await fetchModelIdsLike();
} catch (error) {
console.log(JSON.stringify({
name: error.name,
message: error.message,
catchRan,
endpointSpecific: error.message === 'endpoint-specific timeout',
deadlineAborted: deadline.aborted,
}));
}
})();
JSRepository: akitorahayashi/vlm-chat
Length of output: 5673
🏁 Script executed:
#!/bin/bash
set -eu
node - <<'JS'
const timeoutMs = 25;
const deadlineController = new AbortController();
const deadline = deadlineController.signal;
setTimeout(() => deadlineController.abort(new DOMException('The operation was aborted.', 'AbortError')), timeoutMs);
let catchRan = false;
const response = {
async json() {
await new Promise((resolve, reject) => {
const onAbort = () => reject(deadline.reason);
if (deadline.aborted) return onAbort();
deadline.addEventListener('abort', onAbort, { once: true });
});
return { data: [] };
},
};
async function fetchModelIdsLike() {
try {
await Promise.resolve({ response }); // headers arrive before the deadline
} catch (error) {
catchRan = true;
if (deadline.aborted) throw new Error('endpoint-specific timeout');
throw error;
}
return response.json(); // body read is outside the timeout catch
}
(async () => {
try {
await fetchModelIdsLike();
} catch (error) {
console.log(JSON.stringify({
name: error.name,
message: error.message,
catchRan,
endpointSpecific: error.message === 'endpoint-specific timeout',
deadlineAborted: deadline.aborted,
}));
}
})();
JSRepository: akitorahayashi/vlm-chat
Length of output: 287
Handle model-list body timeouts.
If /v1/models sends headers but stalls before body completion, response.json() rejects outside the timeout handler and bypasses the endpoint-specific timeout error. Include the body read in the same try/catch and add a delayed-body 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/lib/inference/client.ts` around lines 59 - 75, Update fetchModelIds so
the /v1/models response body read, including response.json(), occurs inside the
existing try/catch that handles request timeouts. Preserve the endpoint-specific
timeout error when the deadline aborts during either headers or body completion,
and add a test covering a response whose headers arrive but whose body stalls.
| export function claimCompletion(input: { | ||
| completionId: string; | ||
| conversationId: string; | ||
| controller: AbortController; | ||
| }) { | ||
| if (byConversation.has(input.conversationId)) { | ||
| return false; | ||
| } | ||
|
|
||
| running.set(input.completionId, { | ||
| conversationId: input.conversationId, | ||
| controller: input.controller, | ||
| }); | ||
| byConversation.set(input.conversationId, input.completionId); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject duplicate active completion IDs.
claimCompletion only checks byConversation. A request can reuse an active completionId for a different conversation and overwrite its running entry. Later cancellation or release then targets the wrong completion and leaves the original conversation permanently claimed.
Reject IDs already present in running. Also make release ownership-aware, or retain cancelled IDs until the original generator finishes.
🤖 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/running-completions.ts` around lines 52 - 65, Update claimCompletion
to reject input.completionId values already present in running, in addition to
the existing byConversation check, before writing either map. Make the
corresponding release/cancellation logic ownership-aware so a stale completion
cannot remove or alter a newer claim; preserve the original conversation’s claim
until its generator finishes if cancellation does not immediately release it.
| it('stops a turn that is still waiting for the model to load', async () => { | ||
| // Nothing has been streamed yet and no assistant row exists, so the id the | ||
| // client chose is the only thing that can name the turn. | ||
| const stub = useInferenceStub({ | ||
| chunks: contentOnlyStream, | ||
| headerDelayMs: 500, | ||
| }); | ||
| const completionId = 'cancel-before-start'; | ||
|
|
||
| try { | ||
| const pending = post({ | ||
| completionId, | ||
| modelId: 'stub/model', | ||
| text: 'hi', | ||
| }); | ||
|
|
||
| await Bun.sleep(80); | ||
|
|
||
| expect(await (await cancel(completionId)).json()).toEqual({ | ||
| cancelled: true, | ||
| }); | ||
| expect((await pending).status).toBe(499); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Replace the fixed sleep with a poll on observable state.
Bun.sleep(80) assumes the route reaches openChatCompletion and registers the claim within 80 ms. On a loaded CI runner that assumption can fail. cancel(completionId) then returns { cancelled: false } and the assertion fails for a timing reason, not a behavior reason.
Poll until the cancellation succeeds, bounded by a timeout.
♻️ Proposed fix using a bounded poll
- await Bun.sleep(80);
-
- expect(await (await cancel(completionId)).json()).toEqual({
- cancelled: true,
- });
+ let cancelled = false;
+
+ for (let attempt = 0; attempt < 50 && !cancelled; attempt += 1) {
+ await Bun.sleep(10);
+ cancelled = (await (await cancel(completionId)).json()).cancelled;
+ }
+
+ expect(cancelled).toBe(true);
expect((await pending).status).toBe(499);📝 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.
| it('stops a turn that is still waiting for the model to load', async () => { | |
| // Nothing has been streamed yet and no assistant row exists, so the id the | |
| // client chose is the only thing that can name the turn. | |
| const stub = useInferenceStub({ | |
| chunks: contentOnlyStream, | |
| headerDelayMs: 500, | |
| }); | |
| const completionId = 'cancel-before-start'; | |
| try { | |
| const pending = post({ | |
| completionId, | |
| modelId: 'stub/model', | |
| text: 'hi', | |
| }); | |
| await Bun.sleep(80); | |
| expect(await (await cancel(completionId)).json()).toEqual({ | |
| cancelled: true, | |
| }); | |
| expect((await pending).status).toBe(499); | |
| it('stops a turn that is still waiting for the model to load', async () => { | |
| // Nothing has been streamed yet and no assistant row exists, so the id the | |
| // client chose is the only thing that can name the turn. | |
| const stub = useInferenceStub({ | |
| chunks: contentOnlyStream, | |
| headerDelayMs: 500, | |
| }); | |
| const completionId = 'cancel-before-start'; | |
| try { | |
| const pending = post({ | |
| completionId, | |
| modelId: 'stub/model', | |
| text: 'hi', | |
| }); | |
| let cancelled = false; | |
| for (let attempt = 0; attempt < 50 && !cancelled; attempt += 1) { | |
| await Bun.sleep(10); | |
| cancelled = (await (await cancel(completionId)).json()).cancelled; | |
| } | |
| expect(cancelled).toBe(true); | |
| expect((await pending).status).toBe(499); |
🤖 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/integration/chat-route.test.ts` around lines 277 - 298, Replace the
fixed Bun.sleep(80) in the “stops a turn that is still waiting for the model to
load” test with a bounded poll that repeatedly calls cancel(completionId) until
it returns { cancelled: true }. Add a timeout to fail deterministically if
cancellation never succeeds, then assert the successful cancellation response
and preserve the pending request status assertion.
| await response.body?.cancel(); | ||
|
|
||
| const [, assistant] = await readMessages(conversationId); | ||
|
|
||
| expect(assistant.status).toBe('aborted'); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Wait for the aborted status before asserting it.
response.body?.cancel() only tears down the reader. runCompletion records the aborted outcome afterwards, on its own task. The test reads the rows immediately, so it can observe streaming instead of aborted.
Poll the assistant row until it leaves streaming.
♻️ Proposed fix using a bounded poll
await response.body?.cancel();
- const [, assistant] = await readMessages(conversationId);
-
- expect(assistant.status).toBe('aborted');
+ let assistant = (await readMessages(conversationId))[1];
+
+ for (let attempt = 0; attempt < 50 && assistant.status === 'streaming'; attempt += 1) {
+ await Bun.sleep(10);
+ assistant = (await readMessages(conversationId))[1];
+ }
+
+ expect(assistant.status).toBe('aborted');📝 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.
| await response.body?.cancel(); | |
| const [, assistant] = await readMessages(conversationId); | |
| expect(assistant.status).toBe('aborted'); | |
| await response.body?.cancel(); | |
| let assistant = (await readMessages(conversationId))[1]; | |
| for (let attempt = 0; attempt < 50 && assistant.status === 'streaming'; attempt += 1) { | |
| await Bun.sleep(10); | |
| assistant = (await readMessages(conversationId))[1]; | |
| } | |
| expect(assistant.status).toBe('aborted'); |
🤖 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/integration/chat-route.test.ts` around lines 362 - 366, Update the test
around response.body.cancel() and readMessages so it polls the assistant row
until its status leaves streaming before asserting aborted. Use a bounded wait
to avoid hanging indefinitely, then preserve the existing aborted-status
assertion.
| stopServer: stub.stop, | ||
| stop: () => { | ||
| stub.stop(); | ||
|
|
||
| if (previous === undefined) { | ||
| delete process.env.VLM_CHAT_INFERENCE_URL; | ||
| } else { | ||
| process.env.VLM_CHAT_INFERENCE_URL = previous; | ||
| } | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Bun Server.stop closeActiveConnections return value Promise
💡 Result:
The Bun Server.stop method returns a Promise [1][2][3]. This promise resolves once the server has stopped and every connection has been closed [1][2][3]. When calling server.stop(closeActiveConnections), the closeActiveConnections parameter (a boolean) determines how the server shuts down [4][1][3]: - If set to false (or omitted), the server stops accepting new connections but allows existing in-flight requests and WebSocket connections to complete [1][2][3]. - If set to true, the server immediately terminates all in-flight requests and WebSocket connections while stopping new connections from being accepted [1][2][3].
Citations:
- 1: https://bun.sh/docs/runtime/http/server
- 2: https://bun.com/docs/runtime/http/server
- 3: https://bun.com/docs/runtime/http/server.md
- 4: https://bun.sh/reference/bun/Server/stop
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files 'tests/integration/fixtures/inference-stub.ts' 'tests/integration/chat-route.test.ts'
printf '%s\n' '--- inference stub ---'
sed -n '1,180p' tests/integration/fixtures/inference-stub.ts
printf '%s\n' '--- closed-port test context ---'
sed -n '470,535p' tests/integration/chat-route.test.ts
printf '%s\n' '--- stop usages ---'
rg -n -C 3 'stopServer|\.stop\(\)|server\.stop' tests/integrationRepository: akitorahayashi/vlm-chat
Length of output: 15838
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Bun version declarations ---'
fd -i 'package.json|bun.lockb|bun.lock' . -t f -x sh -c 'echo "--- $1"; rg -n -i "bun|engines|packageManager" "$1" || true' sh {}
printf '%s\n' '--- structured fixture call-site inventory ---'
python3 - <<'PY'
from pathlib import Path
import re
fixture = Path("tests/integration/fixtures/inference-stub.ts").read_text()
print("fixture stop return:", re.search(r"stop:\s*\(\)\s*=>\s*server\.stop\(true\)", fixture) is not None)
print("stopServer alias:", re.search(r"stopServer:\s*stub\.stop", fixture) is not None)
print("wrapper discards stop:", re.search(r"stop:\s*\(\)\s*=>\s*\{\s*stub\.stop\(\);", fixture, re.S) is not None)
for path in Path("tests/integration").rglob("*.ts"):
text = path.read_text()
for match in re.finditer(r"(?P<await>\bawait\s+)?(?P<receiver>\w+)\.(?P<method>stopServer|stop)\(\)", text):
line = text.count("\n", 0, match.start()) + 1
print(f"{path}:{line}: {'awaited' if match.group('await') else 'not awaited'} {match.group('receiver')}.{match.group('method')}()")
PYRepository: akitorahayashi/vlm-chat
Length of output: 6804
Await the inference-stub shutdown at every call site.
Bun 1.3.14 server.stop(true) returns Promise<void>. Await stub.stop() before restoring VLM_CHAT_INFERENCE_URL, and await stopServer() before the closed-port request. Update all fixture cleanup calls to await stop(); making the wrapper async alone does not sequence those calls.
🤖 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/integration/fixtures/inference-stub.ts` around lines 104 - 113, Update
the inference-stub cleanup wrapper around stop to await stub.stop() before
restoring VLM_CHAT_INFERENCE_URL, and make every fixture cleanup call await
stop() (including stopServer() before the closed-port request) so shutdown
completes before subsequent operations.
1 participant
Summary by CodeRabbit
New Features
Documentation
Bug Fixes