Skip to content

fix(daemon): allow app_control_stop to run when client is disconnected#30079

Merged
siddseethepalli merged 1 commit into
mainfrom
swarm/43fc/task-190
May 9, 2026
Merged

fix(daemon): allow app_control_stop to run when client is disconnected#30079
siddseethepalli merged 1 commit into
mainfrom
swarm/43fc/task-190

Conversation

@siddseethepalli
Copy link
Copy Markdown
Contributor

@siddseethepalli siddseethepalli commented May 9, 2026

Summary

  • Move the app_control_stop short-circuit BEFORE the isAvailable() gate so a disconnected client cannot strand the singleton lock.
  • stop is purely local cleanup (dispose proxy, clear conversation reference); it never round-trips to the client and shouldn't require an available one.
  • No-op safely when the proxy is already gone.

Addresses Devin review feedback on #29340.


Open in Devin Review

@siddseethepalli siddseethepalli self-assigned this May 9, 2026
@siddseethepalli siddseethepalli merged commit a92fa3d into main May 9, 2026
@siddseethepalli siddseethepalli deleted the swarm/43fc/task-190 branch May 9, 2026 04:18
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment thread assistant/src/daemon/conversation-surfaces.ts
siddseethepalli added a commit that referenced this pull request May 9, 2026
… no proxy attached (#30086)

PR #30079 moved the app_control_stop short-circuit BEFORE the isAvailable()
gate so a disconnected client cannot strand the singleton lock. After that
change, stop succeeds as a no-op even when no proxy is attached, but this
test still asserted the old isError behavior.

Update the assertion to match the new contract (stop is idempotent).

Co-authored-by: Vellum Assistant <assistant@vellum.ai>
@siddseethepalli
Copy link
Copy Markdown
Contributor Author

Already addressed by #30086 (merged 2026-05-09). The test at conversation-surfaces-app-control.test.ts:160-173 now asserts the new idempotent-success contract for app_control_stop when no proxy is attached.

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