Skip to content

fix: prevent duplicate stop signals - #119

Merged
warunalakshitha merged 3 commits into
ballerina-nutcracker:mainfrom
snelusha:fix/run-lifecycle-panic
Aug 11, 2026
Merged

warunalakshitha merged 3 commits into
ballerina-nutcracker:mainfrom
snelusha:fix/run-lifecycle-panic

Conversation

@snelusha

@snelusha snelusha commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Resolves #117

Summary by CodeRabbit

  • Bug Fixes
    • Improved error messages when attempting to start a program that is already running or stop one that is already stopping or stopped.
    • Repeated stop requests are now handled safely without disrupting the active listener.
    • Listeners can be restarted and stopped normally after repeated stop attempts.
    • Improved run and stop signal handling to prevent duplicate or invalid requests from causing unexpected behavior.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 220da7a3-3e0b-448a-8de2-568fa8708122

📥 Commits

Reviewing files that changed from the base of the PR and between cd7b514 and 2c13517.

📒 Files selected for processing (1)
  • packages/wasm/signals_wasm.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/wasm/signals_wasm.go

📝 Walkthrough

Walkthrough

The change synchronizes listener stop signals, improves runtime error messages, and adds an end-to-end test for repeated stop requests and subsequent listener restart.

Changes

Listener stop handling

Layer / File(s) Summary
Synchronize stop-signal state
packages/wasm/signals_wasm.go
signalSource now protects channel state with a mutex, permits one successful signal send, uses a single-slot buffer, and performs idempotent cleanup.
Update runtime stop handling
apps/web/src/workers/ballerina-worker.ts, packages/wasm/main_wasm.go
The worker distinguishes programs that are stopping or stopped. The WASM runtime reports that a Ballerina program is already running.
Validate repeated stop requests
e2e/tests/listeners.spec.ts
The end-to-end test issues repeated stop requests, verifies the Run state, and confirms that the listener can restart and stop.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: tharmigank, warunalakshitha

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description only states the issue link and omits nearly all required sections, including goals, approach, tests, security checks, and test environment. Complete the required template sections with the implementation, test coverage, release note, documentation impact, security checks, and test environment.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preventing duplicate stop signals in the playground run lifecycle.
Linked Issues check ✅ Passed The changes prevent duplicate stop handling, improve lifecycle errors, and add a restart and stop regression test for issue #117.
Out of Scope Changes check ✅ Passed All changed files support issue #117 by fixing stop-signal handling, correcting errors, or testing repeated run and stop behavior.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
e2e/tests/listeners.spec.ts (1)

42-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that the duplicate-stop path was exercised.

The test can pass when the first click succeeds and the second click is ignored by the UI. Add an observable assertion for the second request, such as its expected stop status or error message. This aligns the regression test with the updated worker contract.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/tests/listeners.spec.ts` around lines 42 - 46, Update the test around
playground.runButton to assert an observable result from the second click, such
as the expected stop status or error message, so it verifies the duplicate-stop
path was exercised rather than merely confirming the button text. Preserve the
existing two-click interaction and Run-button assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@e2e/tests/listeners.spec.ts`:
- Around line 42-46: Update the test around playground.runButton to assert an
observable result from the second click, such as the expected stop status or
error message, so it verifies the duplicate-stop path was exercised rather than
merely confirming the button text. Preserve the existing two-click interaction
and Run-button assertion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 27c3ff0d-e85b-4864-a5ff-345acb9e8e8d

📥 Commits

Reviewing files that changed from the base of the PR and between 9282fba and cd7b514.

📒 Files selected for processing (4)
  • apps/web/src/workers/ballerina-worker.ts
  • e2e/tests/listeners.spec.ts
  • packages/wasm/main_wasm.go
  • packages/wasm/signals_wasm.go

Comment thread packages/wasm/signals_wasm.go Outdated
@warunalakshitha
warunalakshitha merged commit c201980 into ballerina-nutcracker:main Aug 11, 2026
2 checks passed
@snelusha
snelusha deleted the fix/run-lifecycle-panic branch August 12, 2026 04:42
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.

When click Run button multiple times playground get stuck

2 participants