Repository navigation
A worktree owns its ports and its database, plus a functional-review skill - #44
Merged
Merged
Conversation
Several worktrees run at once and each hand-copies the same gitignored .env, so every branch fights over 3000/3001/4000 and shares scibly-dev. `pnpm wt:up` derives a stable port block and database name from the worktree's directory name, writes its five .env files, and migrates and seeds its own database; `pnpm wt:down` tears that back down. See docs/adr/0007-a-worktree-owns-its-ports-and-its-database.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…te step Behavior-preserving. Refactor plan steps 1, 8, 9, 10. - turbo.json: revert five pure-reformatting hunks that made the diff unreviewable (step 1), and move APP_PORT/WEB_PORT out of globalEnv into the `dev` task's own `env` (step 8). globalEnv hashes into *every* cached task, so a worktree's port made typecheck, lint and format:check all miss cache for reasons unrelated to their inputs. `dev` is cache:false, so the scoped entry costs nothing. Verified: `APP_PORT=9999 WEB_PORT=8888 pnpm check` -> 32/32 cached, FULL TURBO. Before, it rebuilt everything. - package.json + ci.yml: one root `generate` script instead of the same three commands written out in both ci.yml and worktree-up.mjs, whose comment admitted the duplication (step 9). Same commands, same order. - docs/setup.md + ADR 0007: setup.md still stated 3000/3001/4000 as fact, which is wrong inside a worktree; the ADR never named the scripts that implement it (step 10). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Refactor plan steps 2-7 and 11, plus behavior-changing fixes B1-B15. The
restructure and the fixes land together because in worktree-lib.mjs they
are literally the same lines: `readDatabaseUrl` moving to `parseEnv` (step
4) *is* the trailing-comment fix (B8); `rewriteEnv` becoming one pass
(step 5) *is* the missed-host fix (B7). Splitting them would mean an
intermediate commit whose own tests fail, so they are separated in this
message instead. Refactor first, then each fix.
Phase 0 — safety net, landed first in the file:
scripts/worktree-lib.test.mjs, 38 node:test cases pinning behavior.
node:test, not vitest: root scripts/ is in no workspace, so no
vitest.config.ts in this repo can reach it. Run with `pnpm test:scripts`.
Tests named FIXED(Bn) record an assertion deliberately flipped by the
fix in this same commit. `// @ts-check` + JSDoc on all three scripts —
editor-only, since root has neither typescript nor @types/node and the
plan forbids adding them.
Refactor (behavior-preserving):
2 Correct three comments describing code that did not exist.
3 mainAdminUrl()/databaseExists() — one home for the db-env path,
read, local-check and admin-URL chain, which both scripts open-coded.
4 readDatabaseUrl() via node:util parseEnv, the same stdlib parser
scripts/dev.mjs already uses.
5 rewriteEnv() is one pass over the text, so no substitution can
re-match another's output.
6 Build launch.json's three configurations from Object.entries(ports)
instead of three near-identical literals. Order becomes web, app,
collab.
7 MAIN_PORTS is module-private; APP_BY_MAIN_PORT derives from it.
11 derive/locate/rewrite -> worktreeConfig/worktreePaths/rewriteEnv.
Behavior-changing fixes:
B1 CRITICAL, had already fired. writeFileSync follows symlinks, and
this worktree's .env and packages/db/.env were symlinks into the
main checkout, so wt:up wrote this worktree's ports and database
name into main's real files — and the read-back saw its own output
through the same link and reported "unchanged". detachFromSource()
unlinks a symlink before writing and hard-fails on a same-inode
target (a hard link, which realpathSync cannot see). Main's two
files were repaired by hand.
B2 assertPortsFree() — the guard an existing comment already claimed
existed. Two live worktrees had in fact been assigned the same
block. Slot space widened from 90 three-port blocks to ~900.
B3 The database digest suffix is now unconditional: feat-x, feat_x,
Feat.x and feat/x all flattened to one database name while getting
different ports, so nothing looked wrong. 10+44+1+8 = 63, the
Postgres identifier limit exactly.
B4 launch.json ports were decorative — apps read ${APP_PORT:-3001},
which only scripts/dev.mjs set, so app booted on the main
checkout's port while the debugger waited on this worktree's. Each
configuration now carries its own <NAME>_PORT.
B5 wt:down SIGTERMed every PID on a derived port with no ownership
check, while the strictly more recoverable database drop prompted.
processCwd() skips a process started outside this worktree.
B6 install now runs before migrate and seed — prisma generate is
@scibly/db's postinstall, and both are prisma invocations.
B7 Port substitution now matches 127.0.0.1, 0.0.0.0 and [::1], not
just localhost, and `(?!\d)` stops localhost:30000 being read as
localhost:3000. A missed host is the dangerous case: dev.mjs then
parses the main port back out and boots on top of it, so rewriteEnv
also refuses to return text still pointing at one.
B8 Trailing comments were glued into DATABASE_URL, and
`export DATABASE_URL=` reported "no DATABASE_URL found".
B9 wt:down refuses a database name not starting with scibly_wt_,
pinning the invariant at the destructive site.
B10 The documented caller (.agents/skills/functional-review) runs as a
subagent with no TTY, where the confirm prompt simply hung. It now
passes --yes, and wt:down refuses rather than assumes when stdin is
not a TTY.
B11 --frozen-lockfile dropped: it refuses the exact stale-lockfile case
its own comment named, aborting after seed had already run.
B12 psql's connection string came back verbatim in Node's "Command
failed:" message, putting credentials in stdout and transcripts.
B13 A missing lsof or psql was indistinguishable from "nothing is
listening"; both now rethrow on ENOENT.
B14 Env files are written 0600 — they carry BETTER_AUTH_SECRET,
COLLAB_TOKEN_SECRET and a credentialed DATABASE_URL.
B15 assertLocal's "::1" entry could never match (Node reports [::1]),
and libpq's ?host=/?hostaddr= silently overrode a local-looking
URL. Both fixed; the ADR now records what the check cannot see.
B2 and B3 change every worktree's ports and database name, so each
re-provisions on its next wt:up and its old scibly_wt_* database is
orphaned.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ything `writeFileSync`'s `mode` applies only when the file is created and is silently ignored when it already exists, so the five env files stayed at 0644 through every rewrite after the first — carrying BETTER_AUTH_SECRET, COLLAB_TOKEN_SECRET and a credentialed DATABASE_URL. chmod explicitly, on the unchanged path too, so a stale mode heals instead of persisting. `wt:down`'s two hard refusals (database-name guard, non-TTY) sat below the SIGTERM loop, so a refused teardown still stopped the dev servers while reporting it had done nothing. Hoist both above the loop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
A worktree database outliving the agent that made it is the design: the stack is left up so a human can open the app and see what a review meant. Outliving the *worktree* is a leak — the directory goes, and nothing points at the database or will ever name it again. `pnpm wt:prune` drops every `scibly_wt_*` database no worktree derives, and `wt:up` runs the same sweep before provisioning, so at most one generation survives without anyone having to remember. Liveness comes from `git worktree list --porcelain` rather than a readdir of the worktrees directory: it is the only source that sees a worktree added elsewhere, and the only one that tells a deleted directory (`prunable`) from one that was never there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lets an agent run a full functional review in an isolated stack, so two worktrees never fight over a port or a database.
pnpm wt:up/pnpm wt:down— three consecutive ports and ascibly_wt_*database, both hashed from the worktree directory name so they never move between runs. Copies all five gitignored.envfiles from the main checkout and rewrites the ports andDATABASE_URLin them; installs, migrates, seeds, generates.wt:downstops the servers and drops the database, behind a name guard and a TTY/--yesconfirmation. A database is meant to outlive the agent that made it, not the worktree:pnpm wt:prunedrops the ones no worktree derives, andwt:upruns that sweep before provisioning./functional-reviewskill — brings the stack up, runspnpm validate, drives the app as a user, writes a report to the gitignored.reviews/<branch>/. Never fixes anything.Design and the alternatives rejected: ADR 0007.
Verified end to end:
pnpm validategreen (32/32 check, 41 script tests, 2725 unit, 3 e2e) with Playwright attaching to the derived ports viareuseExistingServerand nothing binding the 3001/4000 fallbacks;wt:down --yesreleases all three ports and drops the database. Not yet exercised: the skill run start-to-finish by a fresh subagent on its own.🤖 Generated with Claude Code