fix: use a lock to ensure only need to run tunnel just in case multiple go…#5885
fix: use a lock to ensure only need to run tunnel just in case multiple go…#5885michaelneale merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements file-based locking to ensure only one tunnel instance runs across multiple goose-server processes. The fix uses the fs2 crate for cross-process exclusive file locks, preventing duplicate tunnel instances from starting simultaneously.
Key changes:
- Added
fs2dependency for cross-process file locking - Implemented lock acquisition/release functions with PID tracking
- Integrated lock into
TunnelManagerlifecycle (acquire on start, release on stop/error) - Updated status reporting to detect tunnels running in other instances
- Changed auto-start error logging from
errortoinfolevel when lock is already held
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/goose-server/src/tunnel/mod.rs |
Core implementation: adds lock acquisition/checking functions, integrates file lock into TunnelManager struct, updates start/stop to acquire/release lock, modifies status reporting to detect tunnels in other instances |
crates/goose-server/Cargo.toml |
Adds fs2 0.4.3 dependency for file locking |
Cargo.lock |
Updates lock file with fs2 dependency |
alexhancock
left a comment
alexhancock
left a comment
There was a problem hiding this comment.
LGTM if using a file is easiest!
One Q though: is there anything that already must be in use if the tunnel is active? I see there is a port involved somehow. Could you check if something is already using a known port?
|
@alexhancock no known port - the only port used is randomly assigned for goosed (at the moment) - I guess if that was persisted it could check if it is still active and use that as a lock? (I don't think the tunnel itself uses a port - it just internally passes from websocket to the temporary port goosed is using). |
I figured as much - file based seems fine! |
* 'main' of github.com:block/goose: chore: upgrade npm packages (#5951) feat: ActionRequired (#5897) feat(acp): support loading sessions in acp (#5942) docs: add videos to multi-model page (#5938) docs: promote planning guide (#5934) fix: use a lock to ensure only need to run tunnel just in case multiple go… (#5885)
…0-5147 * 'main' of github.com:block/goose: (243 commits) chore: upgrade npm packages (#5951) feat: ActionRequired (#5897) feat(acp): support loading sessions in acp (#5942) docs: add videos to multi-model page (#5938) docs: promote planning guide (#5934) fix: use a lock to ensure only need to run tunnel just in case multiple go… (#5885) Feat: Added custom headers and toggle keyring CLI options (#5017) Feat/automatic update installation (#5345) fix: Added "Merged consecutive assistant messages" to the acceptable issues for moim injection check (#5933) fix: anthropic provider model fetching (#5932) [MCP-UI] add CSP for images to proxy HTML (#5931) fix: correct typo in blog post (AIMDOEL -> AIMODEL) (#5902) feat: @goose in terminal (native terminal support) (#5887) docs: adding AI-friendly features (#5918) Blog/advent of ai announcement (#5917) Extension selector behind ALPHA flag (#5892) blog: typo fixes (#5896) blog: fixing img url (#5895) blog: MCPs for Developers (#5884) docs: Extension Manager MCP (#5883) ... # Conflicts: # crates/goose-server/src/routes/config_management.rs # crates/goose/src/providers/mod.rs # ui/desktop/openapi.json # ui/desktop/src/api/sdk.gen.ts # ui/desktop/src/api/types.gen.ts # ui/desktop/src/components/ProgressiveMessageList.tsx
3 participants
there can be multiple goosed but should only be one tunnel running.
small one - would rather not need to do this, but as things are now, we let many goosed fly (pun intended) so as this is in process, don't really want to double up (not sure what would happen - probably nothing functionally good!)