Skip to content

Fix: Add explicit error when delegate expert not found #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
FL4TLiN3 merged 3 commits into from
Dec 4, 2025
Merged

Fix: Add explicit error when delegate expert not found #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc23849
Fix: Add explicit error when delegate expert not found
FL4TLiN3 Dec 4, 2025
80e2357
Docs: Update CODE_REVIEW_REPORT.md for #34 fix
FL4TLiN3 Dec 4, 2025
3ea1452
Chore: Add changeset for delegate expert error fix
FL4TLiN3 Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-delegate-expert-not-found.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@perstack/runtime": patch
---

Add explicit error message when delegate expert is not found
28 changes: 6 additions & 22 deletions CODE_REVIEW_REPORT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Overall, the codebase is well-structured with strong architectural decisions. Th
| ✅ Verified | 5 | Confirmed not an issue / working correctly |
| 📝 Documented | 1 | Behavior documented, no code change needed |
| ⏸️ Deferred | 8 | Low priority / E2E scope / future work |
| 🔴 Open | 4 | Runtime package issues (2025-12-03) |
| 🔴 Open | 3 | Runtime package issues (2025-12-03) |
| 🟡 Low Prio | 4 | Runtime minor issues (2025-12-03) |

---
Expand Down Expand Up @@ -617,32 +617,16 @@ cd /project && perstack run expert "query"

### 34. Delegate Expert Not Found Causes Runtime Error

**Status**: 🔴 **Open**
**Status**: ✅ **Fixed** — Commit `cc23849`

**Category**: Potential Bug
**Severity**: Minor

**Location**: `packages/runtime/src/skill-manager/helpers.ts:69-74`

**Issue**: When creating DelegateSkillManagers, if a delegate expert is not found in `experts`, `experts[delegateExpertName]` returns `undefined`, causing `new DelegateSkillManager(undefined, ...)` to fail.

```typescript
const delegateSkillManagers = expert.delegates.map((delegateExpertName) => {
const delegate = experts[delegateExpertName] // May be undefined
const manager = new DelegateSkillManager(delegate, runId, eventListener)
// ...
})
```
**Issue**: When creating DelegateSkillManagers, if a delegate expert is not found, error message was unclear.

**Impact**: Unclear error message when delegate expert is missing.

**Recommendation**: Add explicit check with descriptive error:
```typescript
const delegate = experts[delegateExpertName]
if (!delegate) {
throw new Error(`Delegate expert "${delegateExpertName}" not found in experts`)
}
```
**Resolution**: Added explicit check with descriptive error message.

---

Expand Down Expand Up @@ -899,7 +883,7 @@ const metaInstruction = dedent`
| #31 | Skill lazy init for faster startup | ⏸️ Future |
| #32 | maxSteps off-by-one error | ✅ Fixed |
| #33 | Sandbox design flaw (workspace/chdir) | ✅ Fixed |
| #34 | Delegate expert not found error | 🔴 Open |
| #34 | Delegate expert not found error | ✅ Fixed |
| #35 | Skill object mutation | 🔴 Open |
| #36 | experts object mutation | 🔴 Open |
| #37 | File operation error handling | ✅ Fixed |
Expand Down Expand Up @@ -928,7 +912,6 @@ const metaInstruction = dedent`
### Runtime Package Open Issues (2025-12-03)

**Minor (Should Fix)**:
- **#34**: Delegate expert not found — improve error message
- **#35, #36**: Object mutation side effects — defensive copying recommended
- **#38**: RunSetting not validated — use Zod schema

Expand Down Expand Up @@ -957,3 +940,4 @@ const metaInstruction = dedent`
| `ced1aa6` | Fix: maxSteps off-by-one error in finishing step |
| `dac71b5` | Fix: Handle individual close failures in closeSkillManagers |
| `796f981` | Fix: Handle file read errors gracefully in resolving file states |
| `cc23849` | Fix: Add explicit error when delegate expert not found |
3 changes: 3 additions & 0 deletions packages/runtime/src/skill-manager/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ export async function getSkillManagers(
await initSkillManagersWithCleanup(interactiveSkillManagers, allManagers)
const delegateSkillManagers = expert.delegates.map((delegateExpertName) => {
const delegate = experts[delegateExpertName]
if (!delegate) {
throw new Error(`Delegate expert "${delegateExpertName}" not found in experts`)
}
Copy link

@cursor cursor bot Dec 4, 2025

Choose a reason for hiding this comment

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

Bug: Resource leak when delegate expert not found

When a delegate expert is not found, the error is thrown after MCP and interactive skill managers have already been initialized (via initSkillManagersWithCleanup calls at lines 59 and 68). Since the error escapes from within the .map() callback without any cleanup, these already-initialized managers are never closed. This can cause resource leaks such as unclosed MCP server connections. The allManagers array contains initialized managers that need close() called on them before throwing.

Fix in Cursor Fix in Web

const manager = new DelegateSkillManager(delegate, runId, eventListener)
allManagers.push(manager)
return manager
Expand Down