Sign up
View stargazersSign up

fix(server): dev database seed no longer runs settle scripts in real worktrees#33908

open1
main
GitHub fork: t3/dev-seed-settle-scripts
opened8h agobyLLenoridev
2 files changed
View on GitHubTwo-way sync enabledPR last imported 7h ago

Problem

vp run migrate-dev-db, and the automatic seed on a worktree's first vp run dev, copy each project's settle script along with each thread's real worktreePath. When a seeded thread settles on the dev server, the script runs in the developer's real checkout, next to whatever the live install is doing. Automatic settling is on by default, so this happens with no action from the developer.

Fixes #33907.

Change

The prune transaction in apps/server/scripts/migrate-dev-db.ts now removes runOnSettle from every script in projection_projects.scripts_json:

diff
1 pruneSnapshot
2 remove pullRequests[].watch from kept threads
3+ remove runOnSettle from kept projects' scripts
4 clear scheduled tasks, pending work, auth, peer links

The scripts stay listed in the dev UI, and threads keep their worktreePath, so seeded threads still read as real history. A seeded thread still settles normally. It just has no settle script to run.

Scope and approval

This is a one-statement fix for a confirmed bug, so I believe it qualifies as a very small, focused fix. It has its own report in #33907. It closes one more path of the class in #15513, the same way #32744 closed the pull request watch path reported in #32736. The change affects only the dev seed and does not touch a running server.

Verification

Failing test first. The existing "keeps stopped and settled thread families and clears pending work" case now has a project with a plain script and a runOnSettle script, and asserts that both stay and that runOnSettle is gone.

  • Before the fix, it fails: expected [ …(2) ] to deeply equal [ …(2) ], with + "runOnSettle": true in the copied project.
  • After the fix, all 9 tests in migrate-dev-db.test.ts pass.

Runtime transcript from a fixture. All paths were temporary directories, HOME was temporary, and the environment was otherwise empty. I built a source database with runOnSettle set to pwd > "$PWD/settle-script-ran.marker" and one stopped thread idle for more than 7 days. Then I ran the real migrate-dev-db.ts and booted bin.ts serve on the result with default settings and no settings.json. Nothing else acted on the thread.

shell
1# Before the fix (main @ 41443e5706)
2seeded script: runOnWorktreeCreate=false runOnSettle=true
3seeded thread: settledOverride=null hasWorktreePath=1
4thread settled by the automatic sweep: yes
5settledOverride="settled"
6settle script RAN in worktree: $TMP/e2e-before/real-worktree
7
8# After the fix
9seeded script: runOnWorktreeCreate=false (runOnSettle removed)
10seeded thread: settledOverride=null hasWorktreePath=1
11thread settled by the automatic sweep: yes
12settledOverride="settled"
13settle script did NOT run (no marker, checked for 30 seconds after settling)

vp run check:changed: format, lint, GraphQL, typecheck and tests passed. One knip pass hit the script's 900-second limit on this 3.3 GB machine, so I ran the full vp run knip:check through the commit hook, and it passed.

Not checked or not covered:

  • I exercised only the inactivity sweep end to end. Manual settle and settle on merge or close were not run. From the code, both reach the same thread.settled handler.
  • A settings.json copied by hand into a dev home can still carry settle scripts in projectSettingsOverrides or defaultProjectScripts, and those take precedence over scripts_json. The seed does not write that file, and AGENTS.md describes copying it as opt-in. A worktree that started a dev server before this fix may have already folded the old scripts into its own settings.json, and rerunning the seed does not change that file. I read this in the code and did not run it.
  • A turn a developer starts in a seeded thread still runs in that thread's real worktreePath. That is unchanged and outside this fix.

🤖 Generated with Claude Code

Fixes pingdotgg/t3code#33907

Checks

23 successful · 10 skipped
All checks passed
e17622b

Added 1 commit

  • fix(server): dev database seed no longer runs settle scripts in real worktreese17622b
MmacroscopeappBot8h ago
Synced from GitHub

Approvability

Verdict: Approved at e17622b

Macroscope's review found this PR approvable — Development database seeding now strips automatic settle triggers from copied project scripts, avoiding execution in real worktrees. The change is limited to the copied snapshot and leaves source data and product defaults unchanged.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot8h ago
Synced from GitHub

Explain Change →

Walkthrough

The dev-database snapshot pruning now removes runOnSettle from project scripts when present. The migration test verifies that both scripts remain in the copied project data while the copied teardown script omits that property.

Changes: Dev database script pruning

Layer / File(s)Summary
Prune and verify copied scripts
apps/server/scripts/migrate-dev-db.ts, apps/server/scripts/migrate-dev-db.test.ts		pruneSnapshot removes runOnSettle from project scripts when present. The test checks that both scripts remain in the copied project data and that the copied teardown script omits the property.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

  • pingdotgg/t3code#32736 — This issue describes pull-request watches as a separate trigger that this change does not modify.
  • pingdotgg/t3code#33901 — This issue describes worktree cleanup behavior that this change does not modify.

Suggested labels: bugfix, size:XS

Suggested reviewers: juliusmarminge, macroscopeapp

Merge Risk: External image Minimal · up to e1762

Seeded dev databases no longer carry settle-on-completion flags into copied projects, so they stop running scripts in real worktrees on their own. Scripts defined in a developer's own settings file can still run, which the author has noted as out of scope for this change.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title clearly describes the main change: the dev database seed no longer runs settle scripts in real worktrees.
Linked Issues checkPassed[#33907] The migration now removes runOnSettle from copied project scripts while retaining the script entries and other fields. This prevents seeded threads from invoking the settle script in their …
Out of Scope Changes checkPassedThe production change is limited to pruning the settle trigger during dev database seeding. The added test covers that behavior and confirms scripts remain visible. No unrelated changes appear in the …
Description checkPassedThe description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the bug and fix, justifies the focused scope, links the issue, reports test results, a…
Full details: Linked Issues check

Explanation

[#33907] The migration now removes runOnSettle from copied project scripts while retaining the script entries and other fields. This prevents seeded threads from invoking the settle script in their retained real worktree paths. The test verifies that both scripts remain listed and the settle trigger is absent. The issue does not require changing worktree paths or developer-started turns.

Full details: Out of Scope Changes check

Explanation

The production change is limited to pruning the settle trigger during dev database seeding. The added test covers that behavior and confirms scripts remain visible. No unrelated changes appear in the whole-PR diff.

Full details: Description check

Explanation

The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the bug and fix, justifies the focused scope, links the issue, reports test results, and identifies checks that were not performed.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
CcoderabbitaiBotreviewed8h ago
Synced from GitHub
External image Additional comments: 1

apps/server/scripts/migrate-dev-db.ts-392-398: 1

392-398: External image Major · Functional Correctness · Heavy lift

External image Unverified finding Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Disable settle scripts in settings-held script lists.

If the dev server’s settings.json contains a defaultProjectScripts list or a project script override with runOnSettle: true, resolveProjectScripts can select that list instead of the pruned scripts_json. The automatic settlement sweep can then run the script in a copied thread’s real worktree. Disable settle execution for those settings-held lists in the seed workflow as well. (raw.githubusercontent.com)

External image Review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between d313b2aff31991ab94c8ac80cdc38a54dd575604 and e17622b24d982c867daf2f382f44d78692061a32.

External image Files selected for processing: 2

  • apps/server/scripts/migrate-dev-db.test.ts
  • apps/server/scripts/migrate-dev-db.ts

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