Skip to content

fix(scripts): a second dev session no longer takes over the running one - #140

Merged
yordis merged 1 commit into
mainfrom
yordis/fix-dev-runner-owned-state-dir
Oct 22, 2026
Merged

yordis merged 1 commit into
mainfrom
yordis/fix-dev-runner-owned-state-dir

Conversation

@yordis

@yordis yordis commented Oct 22, 2026 •

Copy link
Copy Markdown
Member
  • Starting a second dev session against the same T3 home was refused, but its file watcher stayed alive, so the next file change restarted both servers and the newcomer could win the ownership lock, breaking the session that was already running.
  • The dev session that arrives second should be the one that fails, immediately and without leaving anything behind.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@cursor

cursor Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Scoped to dev-runner startup gating in local development; mirrors existing server lock behavior and does not change production server or auth paths.

Overview
Prevents a second dev / dev:server session from spawning anything when the T3 state directory is already owned by a running server, closing a race where a refused start still left a node --watch parent alive and the next restart could steal the ownership lock.

The dev-runner now derives the same stateDir as the server (userdata vs dev under the resolved home), probes server-owner.sqlite with the same exclusive-transaction logic as serverOwnership.ts, and returns DevRunnerStateDirOwnedError before share setup, first-start prep, or vp spawn. A new test holds an exclusive SQLite lock on disk and asserts the runner fails without calling the process spawner.

Reviewed by Cursor Bugbot for commit a753a4d. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M labels Oct 22, 2026
@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between aa7e127 and a753a4d.

📒 Files selected for processing: 2
  • scripts/dev-runner.test.ts
  • scripts/dev-runner.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The dev runner now checks whether the selected state directory is owned by another server before starting dev or dev:server. If the SQLite lock is busy, it raises DevRunnerStateDirOwnedError. A test verifies that the runner does not spawn a child process in that case.

Changes: State directory ownership guard

Layer / File(s) Summary
Lock detection and startup guard
scripts/dev-runner.ts, scripts/dev-runner.test.ts
The runner checks the server-owner.sqlite lock for non-dry-run dev and dev:server commands. It raises DevRunnerStateDirOwnedError when the lock is busy. The test verifies the error includes the state directory and that no child process is spawned.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to a753a

The change makes a second dev session on the same state directory fail immediately instead of leaving a watcher running. A rare race remains if two sessions start at the same moment. It does not block merging.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the problem and expected behavior, but it omits the required Change, Scope and approval, and Verification sections. Add a Change section describing the SQLite ownership check and error handling. Add Scope and approval details, including the linked issue or approval rationale. Add Verification details with the focused test command and observed results, an…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preventing a second dev session from taking over an existing session.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Resolution

Add a Change section describing the SQLite ownership check and error handling. Add Scope and approval details, including the linked issue or approval rationale. Add Verification details with the focused test command and observed results, and state any checks not run.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@yordis
yordis merged commit bedf0ef into main Oct 22, 2026
27 of 28 checks passed
@yordis
yordis deleted the yordis/fix-dev-runner-owned-state-dir branch October 22, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant