Skip to content

fix: add timeout and cancellation support for notifications and registry fetch#13

Open
HomyeeKing wants to merge 5 commits into
formulahendry:mainfrom
HomyeeKing:fix/notification-timeout-cancellation
Open

fix: add timeout and cancellation support for notifications and registry fetch#13
HomyeeKing wants to merge 5 commits into
formulahendry:mainfrom
HomyeeKing:fix/notification-timeout-cancellation

Conversation

@HomyeeKing

@HomyeeKing HomyeeKing commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add 30s timeout to registry fetch with AbortController
  • Make progress notifications cancellable for connect, new conversation, and restart actions
  • Add cancellation callbacks for better logging
  • Improve error messages to include agent name
image

Test plan

  • Test connection notification can be cancelled
  • Test restart notification can be cancelled
  • Test new conversation notification can be cancelled
  • Test registry fetch times out after 30s
  • Verify error messages include agent name

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds cancellation/timeout plumbing to improve user experience when connecting/restarting agents and when fetching the remote agent registry.

Changes:

  • Make connection/new-conversation/restart progress notifications cancellable and add cancellation logging.
  • Add a 30s timeout to registry fetch via AbortController.
  • Update some error messages to include the agent name.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 7 comments.

File Description
src/extension.ts Makes several progress notifications cancellable and enriches some error messages.
src/config/RegistryClient.ts Adds fetch timeout/cancellation for registry retrieval.
package-lock.json Lockfile metadata churn (peer flags removed).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/extension.ts Outdated
Comment thread src/extension.ts Outdated
Comment thread src/extension.ts Outdated
Comment thread src/extension.ts Outdated
Comment thread src/extension.ts Outdated
Comment thread src/config/RegistryClient.ts Outdated
Comment thread src/extension.ts Outdated
…try fetch

- Add 30s timeout to registry fetch with AbortController
- Make progress notifications cancellable for connect, new conversation, and restart actions
- Add cancellation callbacks for better logging
- Improve error messages to include agent name
@HomyeeKing
HomyeeKing force-pushed the fix/notification-timeout-cancellation branch from 8df3c23 to b4b5bc3 Compare March 24, 2026 11:28
@HomyeeKing

Copy link
Copy Markdown
Contributor Author

@formulahendry ping

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/utils/Logger.ts
Comment thread src/utils/Logger.ts
Comment thread src/extension.ts
Comment thread src/extension.ts
@HomyeeKing

HomyeeKing commented Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

ping @formulahendry

Vanisper added a commit to Vanisper/vscode-acp that referenced this pull request Apr 10, 2026
…nect()

Prevents indefinite hangs during connection initialization by:
- Detecting when agent process exits during connection
- Adding 30-second timeout for connection initialization
- Properly cleaning up streams on error
- Providing clear error messages for failure scenarios

Fixes the issue where ConnectionManager.connect() would hang forever
when the agent process crashes or exits before initialization completes.

Reference: formulahendry#13
Vanisper added a commit to Vanisper/vscode-acp that referenced this pull request Apr 10, 2026
…nect()

Prevents indefinite hangs during connection initialization by:
- Detecting when agent process exits during connection
- Adding 30-second timeout for connection initialization
- Properly cleaning up streams on error
- Providing clear error messages for failure scenarios

Fixes the issue where ConnectionManager.connect() would hang forever
when the agent process crashes or exits before initialization completes.

Reference: formulahendry#13
@HomyeeKing

Copy link
Copy Markdown
Contributor Author

@formulahendry cc

@formulahendry formulahendry left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

For recent 4 comments from Copilot, please help resolve them. If you think some comment is inappropriate, please share the Won't Fix reason.

alanjds pushed a commit to alanjds/vscode-acp that referenced this pull request Jun 30, 2026
Cherry-picked and adapted from formulahendry/vscode-acp:

- fix(formulahendry#46): use correct Copilot package (@github/copilot@latest)
- fix(#2): serialize concurrent permission requests to prevent silent
  cancellations when multiple QuickPick dialogs overlap
- fix(formulahendry#6): respect acp.defaultWorkingDirectory in CWD resolution so
  workspace-scoped agents (kiro-cli etc.) find their config files
- fix(formulahendry#13): add 30s AbortController timeout to registry fetch;
  make connect/newConversation/restart progress notifications cancellable
- fix(formulahendry#47): redirect ACP pipe to fd 3 on login shells to prevent
  profile scripts (e.g. distrobox_profile.sh) from consuming fd 0
- feat(formulahendry#18): security hardening — workspace path validation in
  FileSystemHandler, env var filtering and shell:false in TerminalHandler
- feat(formulahendry#42): editor context link — toggle to prepend active file/cursor
  info to chat prompts (acp.toggleEditorContext command)
- feat(formulahendry#40): add Grok Build as pre-configured agent
- feat(formulahendry#35): add openHarness as pre-configured agent
- feat(formulahendry#44): add siGit Code as pre-configured agent
- feat(formulahendry#37/agent): add Reasonix as pre-configured agent
  (diff preview implementation deferred to separate PR using formulahendry#43 approach)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

3 participants