fix(opencode): guard unwrapResponse array iteration in background manager#968
fix(opencode): guard unwrapResponse array iteration in background manager#968devin-ai-integration[bot] wants to merge 2 commits intomainfrom
Conversation
…ager
When the API returns {} instead of [] for empty session/children lists,
unwrapResponse returns a truthy non-iterable object. The ?? [] fallback
does not trigger because {} is truthy. This causes a TypeError: {} is
not iterable when iterating with for...of.
Replace ?? [] with explicit Array.isArray() guards at the two affected
call sites in recoverTasks() and refreshStatuses(), matching the
existing pattern already used in inspectTask() and fetchLatestResult().
Fixes #966
Co-Authored-By: Rick Blalock <rickblalock@mac.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
📝 WalkthroughWalkthroughDefensive array coercion was added in the background manager: Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
📦 Canary Packages Publishedversion: PackagesInstallAdd to your {
"dependencies": {
"@agentuity/auth": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-auth-1.0.12-ec8a9d6.tgz",
"@agentuity/server": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-server-1.0.12-ec8a9d6.tgz",
"@agentuity/opencode": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-opencode-1.0.12-ec8a9d6.tgz",
"@agentuity/claude-code": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-claude-code-1.0.12-ec8a9d6.tgz",
"@agentuity/workbench": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-workbench-1.0.12-ec8a9d6.tgz",
"@agentuity/runtime": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-runtime-1.0.12-ec8a9d6.tgz",
"@agentuity/react": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-react-1.0.12-ec8a9d6.tgz",
"@agentuity/postgres": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-postgres-1.0.12-ec8a9d6.tgz",
"@agentuity/cli": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-cli-1.0.12-ec8a9d6.tgz",
"@agentuity/frontend": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-frontend-1.0.12-ec8a9d6.tgz",
"@agentuity/evals": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-evals-1.0.12-ec8a9d6.tgz",
"@agentuity/schema": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-schema-1.0.12-ec8a9d6.tgz",
"@agentuity/core": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-core-1.0.12-ec8a9d6.tgz",
"@agentuity/drizzle": "https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-drizzle-1.0.12-ec8a9d6.tgz"
}
}Or install directly: bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-auth-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-server-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-opencode-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-claude-code-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-workbench-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-runtime-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-react-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-postgres-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-cli-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-frontend-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-evals-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-schema-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-core-1.0.12-ec8a9d6.tgz
bun add https://agentuity-sdk-objects.t3.storageapi.dev/npm/1.0.12-ec8a9d6/agentuity-drizzle-1.0.12-ec8a9d6.tgz |
Tests recoverTasks and refreshStatuses handle cases where the API
returns {} instead of [] — the exact scenario that caused the TypeError
in #966. Also tests undefined, null, valid array, valid session
recovery, and non-bg_ task filtering.
Co-Authored-By: Rick Blalock <rickblalock@mac.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/opencode/test/background.test.ts`:
- Around line 273-291: The tests currently define a hand-rolled createMockCtx
that returns a PluginInput stub; replace this with the shared mock from
`@agentuity/test-utils` (e.g., import the library's PluginInput/client/session
mock factory) and use its factory to construct the PluginInput used in tests,
wiring the optional overrides for session.list and session.children into the
factory instead of manually stubbing in createMockCtx so the test reuses the
canonical mocks and behavior provided by `@agentuity/test-utils`.
- Line 5: The test currently imports the PluginInput type from the published
package; change the import to use the local test re-export so tests depend on
../src instead of the package. Update the import statement that references
"PluginInput" in packages/opencode/test/background.test.ts to import the type
from the local ../src re-export (import type { PluginInput } from '../src') so
the test follows the project rule of importing test dependencies from ../src.
- Around line 377-405: The test currently instantiates two BackgroundManager
instances so the one configured with sessionChildren: async () => ({ data: {} })
never runs recoverTasks; change the test to create a single mock context that
returns a sessionList (with the task meta) and a non-array sessionChildren
payload ({ data: {} }), instantiate one BackgroundManager with that context (use
BackgroundManager mgr), call mgr.recoverTasks() and then await
mgr.refreshStatuses(), and assert results; this ensures
BackgroundManager.recoverTasks and BackgroundManager.refreshStatuses exercise
the non-array children path.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/opencode/test/background.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
packages/opencode/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
Use Zod for schema validation
Files:
packages/opencode/test/background.test.ts
packages/opencode/test/**/*.ts
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
When running tests, prefer using a subagent (Task tool) to avoid context bloat from test output
Files:
packages/opencode/test/background.test.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with tabs (width 3), single quotes, and semicolons for code formatting
Files:
packages/opencode/test/background.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with ESNext target and bundler moduleResolution
Files:
packages/opencode/test/background.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
StructuredErrorfrom@agentuity/corefor error handling
Files:
packages/opencode/test/background.test.ts
**/test/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.{test,spec}.{ts,tsx,js,jsx}: Place tests intest/folder, never insrc/or__tests__/directories
Import test dependencies from../src/in test files
Use@agentuity/test-utilsfor mocks in tests
Files:
packages/opencode/test/background.test.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T00:31:41.858Z
Learnt from: jhaynie
Repo: agentuity/sdk PR: 274
File: packages/cli/src/cmd/build/vite/server-bundler.ts:12-41
Timestamp: 2025-12-21T00:31:41.858Z
Learning: In Bun runtime, BuildMessage and ResolveMessage are global types and are not exported from the bun module. Do not import { BuildMessage } from 'bun' or similar; these types are available globally and should be used without import. This applies to all TypeScript files that target the Bun runtime within the repository.
Applied to files:
packages/opencode/test/background.test.ts
🧬 Code graph analysis (1)
packages/opencode/test/background.test.ts (4)
packages/opencode/src/types.ts (1)
PluginInput(9-9)packages/opencode/src/index.ts (1)
PluginInput(9-9)packages/opencode/src/background/manager.ts (1)
BackgroundManager(44-681)packages/opencode/src/background/index.ts (1)
BackgroundManager(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Build
- GitHub Check: Pack & Upload
- GitHub Check: Package Installation & Usage Test
- GitHub Check: Sandbox CLI Tests
- GitHub Check: Framework Integration Tests (TanStack & Next.js)
- GitHub Check: Playwright E2E Smoke Test
- GitHub Check: Queue CLI Tests
- GitHub Check: Cloud Deployment Tests
- GitHub Check: Template Integration Tests
- GitHub Check: SDK Integration Test Suite
- GitHub Check: Postgres SSL Integration Test
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| import { ConcurrencyManager } from '../src/background/concurrency'; | ||
| import { BackgroundManager } from '../src/background/manager'; | ||
| import type { BackgroundTask, BackgroundTaskStatus, TaskProgress } from '../src/background/types'; | ||
| import type { PluginInput } from '@opencode-ai/plugin'; |
There was a problem hiding this comment.
Import PluginInput from local src in tests.
Using the package import bypasses the local export and violates the test dependency rule. Prefer the local re-export in ../src.
As per coding guidelines: "Import test dependencies from ../src/ in test files".
Suggested change
-import type { PluginInput } from '@opencode-ai/plugin';
+import type { PluginInput } from '../src';📝 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.
| import type { PluginInput } from '@opencode-ai/plugin'; | |
| import type { PluginInput } from '../src'; |
🤖 Prompt for AI Agents
In `@packages/opencode/test/background.test.ts` at line 5, The test currently
imports the PluginInput type from the published package; change the import to
use the local test re-export so tests depend on ../src instead of the package.
Update the import statement that references "PluginInput" in
packages/opencode/test/background.test.ts to import the type from the local
../src re-export (import type { PluginInput } from '../src') so the test follows
the project rule of importing test dependencies from ../src.
| function createMockCtx(overrides?: { | ||
| sessionList?: () => Promise<unknown>; | ||
| sessionChildren?: () => Promise<unknown>; | ||
| }): PluginInput { | ||
| return { | ||
| client: { | ||
| session: { | ||
| list: overrides?.sessionList ?? (async () => ({ data: [] })), | ||
| children: overrides?.sessionChildren ?? (async () => ({ data: [] })), | ||
| get: async () => ({ data: {} }), | ||
| messages: async () => ({ data: [] }), | ||
| create: async () => ({ data: { id: 'sess_1' } }), | ||
| prompt: async () => ({}), | ||
| abort: async () => ({}), | ||
| status: async () => ({ data: {} }), | ||
| }, | ||
| }, | ||
| } as unknown as PluginInput; | ||
| } |
There was a problem hiding this comment.
Use @agentuity/test-utils for mocks instead of hand-rolled stubs.
createMockCtx manually constructs a mock, but tests should use the shared mock utilities to keep behavior consistent.
As per coding guidelines: "Use @agentuity/test-utils for mocks in tests".
🤖 Prompt for AI Agents
In `@packages/opencode/test/background.test.ts` around lines 273 - 291, The tests
currently define a hand-rolled createMockCtx that returns a PluginInput stub;
replace this with the shared mock from `@agentuity/test-utils` (e.g., import the
library's PluginInput/client/session mock factory) and use its factory to
construct the PluginInput used in tests, wiring the optional overrides for
session.list and session.children into the factory instead of manually stubbing
in createMockCtx so the test reuses the canonical mocks and behavior provided by
`@agentuity/test-utils`.
| describe('refreshStatuses', () => { | ||
| it('does not throw when session.children returns an empty object instead of an array', async () => { | ||
| const ctx = createMockCtx({ | ||
| sessionChildren: async () => ({ data: {} }), | ||
| }); | ||
| const mgr = new BackgroundManager(ctx); | ||
|
|
||
| const taskMeta = JSON.stringify({ | ||
| taskId: 'bg_test1', | ||
| agent: 'scout', | ||
| description: 'test', | ||
| }); | ||
| const listCtx = createMockCtx({ | ||
| sessionList: async () => ({ | ||
| data: [ | ||
| { | ||
| id: 'sess_1', | ||
| title: taskMeta, | ||
| parentID: 'parent_1', | ||
| status: { type: 'running' }, | ||
| }, | ||
| ], | ||
| }), | ||
| }); | ||
| const listMgr = new BackgroundManager(listCtx); | ||
| await listMgr.recoverTasks(); | ||
|
|
||
| const results = await mgr.refreshStatuses(); | ||
| expect(results).toBeDefined(); |
There was a problem hiding this comment.
refreshStatuses test doesn’t exercise the non-array children path.
The manager that uses sessionChildren: () => ({ data: {} }) never recovers tasks, so session.children is likely never called. Use a single manager that both recovers tasks and returns a non-array children payload.
Suggested change
- it('does not throw when session.children returns an empty object instead of an array', async () => {
- const ctx = createMockCtx({
- sessionChildren: async () => ({ data: {} }),
- });
- const mgr = new BackgroundManager(ctx);
-
- const taskMeta = JSON.stringify({
+ it('does not throw when session.children returns an empty object instead of an array', async () => {
+ const taskMeta = JSON.stringify({
taskId: 'bg_test1',
agent: 'scout',
description: 'test',
});
- const listCtx = createMockCtx({
+ const ctx = createMockCtx({
sessionList: async () => ({
data: [
{
id: 'sess_1',
title: taskMeta,
parentID: 'parent_1',
status: { type: 'running' },
},
],
}),
+ sessionChildren: async () => ({ data: {} }),
});
- const listMgr = new BackgroundManager(listCtx);
- await listMgr.recoverTasks();
+ const mgr = new BackgroundManager(ctx);
+ await mgr.recoverTasks();
const results = await mgr.refreshStatuses();
expect(results).toBeDefined();
});📝 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.
| describe('refreshStatuses', () => { | |
| it('does not throw when session.children returns an empty object instead of an array', async () => { | |
| const ctx = createMockCtx({ | |
| sessionChildren: async () => ({ data: {} }), | |
| }); | |
| const mgr = new BackgroundManager(ctx); | |
| const taskMeta = JSON.stringify({ | |
| taskId: 'bg_test1', | |
| agent: 'scout', | |
| description: 'test', | |
| }); | |
| const listCtx = createMockCtx({ | |
| sessionList: async () => ({ | |
| data: [ | |
| { | |
| id: 'sess_1', | |
| title: taskMeta, | |
| parentID: 'parent_1', | |
| status: { type: 'running' }, | |
| }, | |
| ], | |
| }), | |
| }); | |
| const listMgr = new BackgroundManager(listCtx); | |
| await listMgr.recoverTasks(); | |
| const results = await mgr.refreshStatuses(); | |
| expect(results).toBeDefined(); | |
| describe('refreshStatuses', () => { | |
| it('does not throw when session.children returns an empty object instead of an array', async () => { | |
| const taskMeta = JSON.stringify({ | |
| taskId: 'bg_test1', | |
| agent: 'scout', | |
| description: 'test', | |
| }); | |
| const ctx = createMockCtx({ | |
| sessionList: async () => ({ | |
| data: [ | |
| { | |
| id: 'sess_1', | |
| title: taskMeta, | |
| parentID: 'parent_1', | |
| status: { type: 'running' }, | |
| }, | |
| ], | |
| }), | |
| sessionChildren: async () => ({ data: {} }), | |
| }); | |
| const mgr = new BackgroundManager(ctx); | |
| await mgr.recoverTasks(); | |
| const results = await mgr.refreshStatuses(); | |
| expect(results).toBeDefined(); | |
| }); |
🤖 Prompt for AI Agents
In `@packages/opencode/test/background.test.ts` around lines 377 - 405, The test
currently instantiates two BackgroundManager instances so the one configured
with sessionChildren: async () => ({ data: {} }) never runs recoverTasks; change
the test to create a single mock context that returns a sessionList (with the
task meta) and a non-array sessionChildren payload ({ data: {} }), instantiate
one BackgroundManager with that context (use BackgroundManager mgr), call
mgr.recoverTasks() and then await mgr.refreshStatuses(), and assert results;
this ensures BackgroundManager.recoverTasks and
BackgroundManager.refreshStatuses exercise the non-array children path.
fix(opencode): guard unwrapResponse array iteration in background manager
Summary
Fixes #966. The
@agentuity/opencodeplugin crashes on startup becauserecoverTasks()andrefreshStatuses()iterate over the result ofunwrapResponse()usingfor...of, but the API can return{}(empty object) instead of[]when there are no sessions. The?? []fallback doesn't trigger because{}is truthy, causingTypeError: {} is not iterable.Replaced
?? []with explicitArray.isArray()guards at the two affected call sites (recoverTasksline 243,refreshStatusesline 186), matching the defensive pattern already used at two other sites in the same file (inspectTaskline 141,fetchLatestResultline 636).Updates since last revision
Added unit tests for
BackgroundManagercovering the exact bug scenario and related edge cases:recoverTaskswith{},undefined,null, empty array, valid session data, and non-bg_prefixed task filteringrefreshStatuseswith{}fromsession.childrenAll 29 background tests pass (7 new tests added). Tests use a mock
PluginInputclient to simulate the API returning non-array responses.Review & Testing Checklist for Human
opencode:latest, start the server, and confirm the first message no longer fails with "Unknown error" — this was not tested end-to-end, only via unit tests with mocked clients{ data: {} }to simulate the real API response shape — confirm this matches the actual structure returned by the OpenCode session API when no sessions existunwrapResponseitself: this fix guards the call sites, butunwrapResponsecan still return{}for any future array-expecting caller. Evaluate whether adding an array-aware variant or option would be safer long-term.Notes
Summary by CodeRabbit
Bug Fixes
Tests