Skip to content

Fix: Handle individual close failures in closeSkillManagers#5

Merged
FL4TLiN3 merged 4 commits intomainfrom
fix/close-skill-managers-error-handling
Dec 4, 2025
Merged

Fix: Handle individual close failures in closeSkillManagers#5
FL4TLiN3 merged 4 commits intomainfrom
fix/close-skill-managers-error-handling

Conversation

@FL4TLiN3
Copy link
Contributor

@FL4TLiN3 FL4TLiN3 commented Dec 4, 2025

Summary

  • Fix closeSkillManagers to continue closing other managers when one fails
  • Previously, if one manager's close() threw, subsequent managers were not closed
  • Now uses Promise.all with .catch() to ensure all managers are closed

Test plan

  • Added test case for close failure scenario
  • Existing tests pass
  • Typecheck passes

Note

Make closeSkillManagers close all managers even if some close() calls fail, with tests added and docs/report updated.

  • Runtime
    • Update packages/runtime/src/skill-manager/helpers.ts: closeSkillManagers now uses Promise.all(...m.close().catch(() => {})) to continue closing when a manager fails.
  • Tests
    • Extend packages/runtime/src/skill-manager.test.ts with cases verifying all managers are closed and that closing proceeds when one fails.
  • Docs
    • AGENTS.md: add mandatory E2E testing section and pre-push checklist item.
    • CODE_REVIEW_REPORT.md: update counts; mark issue #39 as fixed and add commit dac71b5.
  • Changeset
    • Add .changeset/fix-close-skill-managers.md (patch for @perstack/runtime).

Written by Cursor Bugbot for commit 6dda9ff. This will update automatically on new commits. Configure here.

@vercel
Copy link

vercel bot commented Dec 4, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
perstack Ready Ready Preview Comment Dec 4, 2025 1:28am

@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant