fix(repos,ui): pin user-supplied default_branch end-to-end + don't reset typed sourceBranch on WebSocket events#1127
Merged
Conversation
added 4 commits
May 8, 2026 15:08
Node 25 exposes a `globalThis.localStorage` stub that lacks the standard Storage methods (no `setItem`/`getItem`/etc.), so the Feathers auth client throws `_a.setItem is not a function` on first authenticate(). This blocks any in-tree daemon / executor / CLI run on a Node 25 host — `createClient()` configures authentication with that broken storage and the very first auth attempt blows up. `createClient()` now treats the global as Storage only when its `setItem` is callable; otherwise it passes `storage: undefined` and the Feathers auth client falls back to its in-memory mode (correct for any non-browser runtime). Tests: - "should reject a localStorage stub without setItem (Node 25)" - "should reject a localStorage stub whose setItem is not a function" Same broken global also bleeds into the agor-ui test runner when the shell exports NODE_OPTIONS=--localstorage-file=..., displacing jsdom's real Storage implementation. apps/agor-ui/src/test/setup.ts now installs an in-memory Storage shim under the same guard, so component tests that read localStorage.getItem(...) work on Node 25 without operators having to unset the env var.
The "Add Repository → Default Branch" form lets the operator pick a
non-default base branch (e.g. a long-lived feature branch). Pre-fix,
that field was silently dropped at every layer and the executor wrote
whatever `origin/HEAD` pointed at into the DB record:
UI sends `default_branch` to /repos/clone
└─ daemon route declared `{ url, name?, destination? }` only
└─ ReposService.cloneRepository signature took the same shape
└─ Executor params did not include `default_branch`
└─ Executor wrote `cloneResult.defaultBranch`
= `git symbolic-ref refs/remotes/origin/HEAD` = "main"
Two visible symptoms:
- Repo's `default_branch` in the DB always equals the upstream's
HEAD, no matter what the operator typed (and re-opening the edit
dialog kept showing "main", looking like the edit silently failed).
- Worktrees created off that repo defaulted to "main" too, so the
"preserved my typed sourceBranch" UI fix from a sibling commit had
nothing useful to fall back to either.
There was also a related second-order bug: even when `default_branch`
finally reached the executor, `cloneRepo()` still ran a default
checkout of the remote's HEAD. Repos whose `.agor.yml` lives on a
non-default branch would be cloned with the file missing on disk, and
the daemon's environment-variant ingestion logged "No environment
variants configured" even though the operator had picked the right
branch.
Threads `default_branch` end-to-end:
- apps/agor-daemon/src/register-routes.ts: add `default_branch?` to
POST /repos/clone body type.
- apps/agor-daemon/src/services/repos.ts: pipe `data.default_branch`
into the executor `git.clone` params (only when set, so existing
`getDefaultBranch()` fallback is preserved).
- packages/executor/src/payload-types.ts: add optional `default_branch`
to `GitClonePayloadSchema.params`.
- packages/executor/src/commands/git.ts:
- forward as `branch` to `cloneRepo()` so the working tree lands
on the pinned branch (fixes the .agor.yml-not-found case);
- prefer `payload.params.default_branch` when writing the repo DB
record, falling back to `cloneResult.defaultBranch` only when
unset (keeps existing behavior for un-pinned clones);
- echo the field in the dry-run response so callers can verify
the field actually reached the handler.
- packages/core/src/git/index.ts: `CloneOptions.branch` opt;
`cloneRepo()` forwards it as `git clone --branch <name>` and
sets `defaultBranch` in the result to the pin (so the DB record
matches what's on disk).
Tests:
- cloneRepo: should check out the pinned branch when options.branch
is set / fall back to remote HEAD when it isn't / fail loudly when
the pin doesn't exist on the remote.
- GitClonePayloadSchema: accept default_branch in params; treat it
as optional.
- executeCommand git.clone: echo user-supplied default_branch in
dry-run response.
Agor real-time-syncs repo metadata over the FeathersJS WebSocket. Every
`repos.patched` event hands the modal/tab a NEW `repoById` Map
reference, which re-fired the form-init `useEffect`. The effect then
called `setFieldsValue({ sourceBranch: repo.default_branch })` and
silently overwrote whatever the operator had typed.
End-user symptom: type a non-default branch, wait a few seconds for any
`repos.patched` event, click Create — the worktree lands on `main`
anyway. No toast, no console warning, just a wrong base branch.
Same anti-pattern in two surfaces — guard both with a useRef so init
runs exactly once per modal-open / mount session:
- NewWorktreeModal.tsx (open-prop modal): the ref resets on close so
re-opening always re-initializes from the fresh repo metadata.
- WorktreeTab.tsx (tab inside CreateDialog): mounts/unmounts with
the dialog, so the per-mount ref is sufficient.
Other potential fields with the same pattern were left alone — the
guard is specifically for sourceBranch which a user types directly;
fields like repoId only change via explicit dropdown handlers
(handleRepoChange) which are intentionally still allowed to reset
sourceBranch.
This is the same shape of fix as #1001 (session list filters being
reset by WebSocket events), applied to a different code path.
Tests (regression coverage):
- NewWorktreeModal: preserves typed sourceBranch across repoById
Map reference churn / re-initializes on close-and-reopen.
- WorktreeTab: preserves typed sourceBranch across repoById churn.
Three followups from a code review of #1127. The PR's primary fixes are correct, but each one had an adjacent surface where the same root cause still bit users. 1. cloneRepo's existing-repo early-return ignored options.branch. When `~/.agor/repos/<slug>` already exists (re-clone after a half- broken first attempt, manual provisioning, restart loop), cloneRepo returned early with `defaultBranch = await getDefaultBranch(targetPath)`, leaving the working tree on whatever was previously checked out. The executor then wrote the user-supplied pin into the DB record. Net: DB claimed `feat/x`, disk on `main`, `.agor.yml` parsed at `cloneResult.path` came from `main` — exactly the symptom the --branch fix was supposed to close. Now: when the existing checkout is on a different branch than the pin, fetch origin/<pin> and check out. Failure (dirty working tree, branch missing on remote) throws with a clear message instead of silently returning a stale defaultBranch. New tests in cloneRepo: - switches the working tree on a reused clone with a pinned branch - rejects reuse when the pin can't be checked out 2. Settings → Worktrees → Create Worktree had the same useEffect anti- pattern as NewWorktreeModal / WorktreeTab. `useEffect([..., repos, boards, ...])` where `repos` and `boards` are derived via `mapToArray(repoById)` / `mapToArray(boardById)` on every render. WebSocket-triggered Map ref churn re-fired the effect and `setFieldsValue({ sourceBranch })` overwrote typed values. Same fix shape as the rest of the PR — useRef gate so init runs exactly once per `createModalOpen=true` session. New test mirroring NewWorktreeModal / WorktreeTab tests. 3. Surface the pinned branch in the cloneError message. The clone exits non-zero with no useful detail when the operator typo'd the Default Branch field — `git clone --branch <typo>` returns 128, the executor's stderr is consumed by spawnExecutorFireAndForget, and the user gets `Clone failed (exit code 128). Check that the repository URL is correct and accessible.` with no hint that the branch is the cause. When a default_branch was supplied, append it to the error so the operator can self-diagnose without diving into daemon logs. (Data integrity is fine; cloneRepo throwing means the executor never reaches the DB-write call site, so no half-formed repo records sneak through. The cleanup is purely about the UX of the error message reaching the UI.)
dhavli
pushed a commit
to Code-Fixxers/agor
that referenced
this pull request
May 12, 2026
…set typed sourceBranch on WebSocket events (preset-io#1127) * fix(core): handle Node 25's broken localStorage global Node 25 exposes a `globalThis.localStorage` stub that lacks the standard Storage methods (no `setItem`/`getItem`/etc.), so the Feathers auth client throws `_a.setItem is not a function` on first authenticate(). This blocks any in-tree daemon / executor / CLI run on a Node 25 host — `createClient()` configures authentication with that broken storage and the very first auth attempt blows up. `createClient()` now treats the global as Storage only when its `setItem` is callable; otherwise it passes `storage: undefined` and the Feathers auth client falls back to its in-memory mode (correct for any non-browser runtime). Tests: - "should reject a localStorage stub without setItem (Node 25)" - "should reject a localStorage stub whose setItem is not a function" Same broken global also bleeds into the agor-ui test runner when the shell exports NODE_OPTIONS=--localstorage-file=..., displacing jsdom's real Storage implementation. apps/agor-ui/src/test/setup.ts now installs an in-memory Storage shim under the same guard, so component tests that read localStorage.getItem(...) work on Node 25 without operators having to unset the env var. * fix(repos): pin user-supplied default_branch end-to-end through clone The "Add Repository → Default Branch" form lets the operator pick a non-default base branch (e.g. a long-lived feature branch). Pre-fix, that field was silently dropped at every layer and the executor wrote whatever `origin/HEAD` pointed at into the DB record: UI sends `default_branch` to /repos/clone └─ daemon route declared `{ url, name?, destination? }` only └─ ReposService.cloneRepository signature took the same shape └─ Executor params did not include `default_branch` └─ Executor wrote `cloneResult.defaultBranch` = `git symbolic-ref refs/remotes/origin/HEAD` = "main" Two visible symptoms: - Repo's `default_branch` in the DB always equals the upstream's HEAD, no matter what the operator typed (and re-opening the edit dialog kept showing "main", looking like the edit silently failed). - Worktrees created off that repo defaulted to "main" too, so the "preserved my typed sourceBranch" UI fix from a sibling commit had nothing useful to fall back to either. There was also a related second-order bug: even when `default_branch` finally reached the executor, `cloneRepo()` still ran a default checkout of the remote's HEAD. Repos whose `.agor.yml` lives on a non-default branch would be cloned with the file missing on disk, and the daemon's environment-variant ingestion logged "No environment variants configured" even though the operator had picked the right branch. Threads `default_branch` end-to-end: - apps/agor-daemon/src/register-routes.ts: add `default_branch?` to POST /repos/clone body type. - apps/agor-daemon/src/services/repos.ts: pipe `data.default_branch` into the executor `git.clone` params (only when set, so existing `getDefaultBranch()` fallback is preserved). - packages/executor/src/payload-types.ts: add optional `default_branch` to `GitClonePayloadSchema.params`. - packages/executor/src/commands/git.ts: - forward as `branch` to `cloneRepo()` so the working tree lands on the pinned branch (fixes the .agor.yml-not-found case); - prefer `payload.params.default_branch` when writing the repo DB record, falling back to `cloneResult.defaultBranch` only when unset (keeps existing behavior for un-pinned clones); - echo the field in the dry-run response so callers can verify the field actually reached the handler. - packages/core/src/git/index.ts: `CloneOptions.branch` opt; `cloneRepo()` forwards it as `git clone --branch <name>` and sets `defaultBranch` in the result to the pin (so the DB record matches what's on disk). Tests: - cloneRepo: should check out the pinned branch when options.branch is set / fall back to remote HEAD when it isn't / fail loudly when the pin doesn't exist on the remote. - GitClonePayloadSchema: accept default_branch in params; treat it as optional. - executeCommand git.clone: echo user-supplied default_branch in dry-run response. * fix(ui): preserve typed sourceBranch across WebSocket repo updates Agor real-time-syncs repo metadata over the FeathersJS WebSocket. Every `repos.patched` event hands the modal/tab a NEW `repoById` Map reference, which re-fired the form-init `useEffect`. The effect then called `setFieldsValue({ sourceBranch: repo.default_branch })` and silently overwrote whatever the operator had typed. End-user symptom: type a non-default branch, wait a few seconds for any `repos.patched` event, click Create — the worktree lands on `main` anyway. No toast, no console warning, just a wrong base branch. Same anti-pattern in two surfaces — guard both with a useRef so init runs exactly once per modal-open / mount session: - NewWorktreeModal.tsx (open-prop modal): the ref resets on close so re-opening always re-initializes from the fresh repo metadata. - WorktreeTab.tsx (tab inside CreateDialog): mounts/unmounts with the dialog, so the per-mount ref is sufficient. Other potential fields with the same pattern were left alone — the guard is specifically for sourceBranch which a user types directly; fields like repoId only change via explicit dropdown handlers (handleRepoChange) which are intentionally still allowed to reset sourceBranch. This is the same shape of fix as preset-io#1001 (session list filters being reset by WebSocket events), applied to a different code path. Tests (regression coverage): - NewWorktreeModal: preserves typed sourceBranch across repoById Map reference churn / re-initializes on close-and-reopen. - WorktreeTab: preserves typed sourceBranch across repoById churn. * fix(repos,ui): close gaps in default_branch end-to-end coverage Three followups from a code review of preset-io#1127. The PR's primary fixes are correct, but each one had an adjacent surface where the same root cause still bit users. 1. cloneRepo's existing-repo early-return ignored options.branch. When `~/.agor/repos/<slug>` already exists (re-clone after a half- broken first attempt, manual provisioning, restart loop), cloneRepo returned early with `defaultBranch = await getDefaultBranch(targetPath)`, leaving the working tree on whatever was previously checked out. The executor then wrote the user-supplied pin into the DB record. Net: DB claimed `feat/x`, disk on `main`, `.agor.yml` parsed at `cloneResult.path` came from `main` — exactly the symptom the --branch fix was supposed to close. Now: when the existing checkout is on a different branch than the pin, fetch origin/<pin> and check out. Failure (dirty working tree, branch missing on remote) throws with a clear message instead of silently returning a stale defaultBranch. New tests in cloneRepo: - switches the working tree on a reused clone with a pinned branch - rejects reuse when the pin can't be checked out 2. Settings → Worktrees → Create Worktree had the same useEffect anti- pattern as NewWorktreeModal / WorktreeTab. `useEffect([..., repos, boards, ...])` where `repos` and `boards` are derived via `mapToArray(repoById)` / `mapToArray(boardById)` on every render. WebSocket-triggered Map ref churn re-fired the effect and `setFieldsValue({ sourceBranch })` overwrote typed values. Same fix shape as the rest of the PR — useRef gate so init runs exactly once per `createModalOpen=true` session. New test mirroring NewWorktreeModal / WorktreeTab tests. 3. Surface the pinned branch in the cloneError message. The clone exits non-zero with no useful detail when the operator typo'd the Default Branch field — `git clone --branch <typo>` returns 128, the executor's stderr is consumed by spawnExecutorFireAndForget, and the user gets `Clone failed (exit code 128). Check that the repository URL is correct and accessible.` with no hint that the branch is the cause. When a default_branch was supplied, append it to the error so the operator can self-diagnose without diving into daemon logs. (Data integrity is fine; cloneRepo throwing means the executor never reaches the DB-write call site, so no half-formed repo records sneak through. The cleanup is purely about the UX of the error message reaching the UI.) --------- Co-authored-by: Diego Pucci <geido@192.168.0.21>
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
2 participants
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.
Summary
End-to-end fix for the "Add Repository → Default Branch" form silently being ignored, plus the worktree-create modal resetting your typed sourceBranch mid-session. Three logically-separable fixes in one PR (one commit each):
Node 25 compat —
createClient()rejects Node 25's brokenglobalThis.localStoragestub (nosetItem) so the Feathers auth client doesn't throw_a.setItem is not a functionon first authenticate. Same fix inapps/agor-ui/src/test/setup.tsso component tests work whenNODE_OPTIONS=--localstorage-file=...is set.default_branchend-to-end through clone — the field was declared on the UI form but dropped at every layer (route signature → service signature → executor params → executor handler), so the DB always wrote whateverorigin/HEADresolved to. Threaded through every layer;cloneRepo()also forwards the pin togit clone --branch <name>so the working tree lands on that branch (otherwise.agor.ymlon a non-default branch wouldn't be visible at parse time).Worktree-create form preserves typed
sourceBranch— everyrepos.patchedWebSocket event handed the parent component a newrepoByIdMap reference; the form-inituseEffectre-fired andsetFieldsValuesilently overwrote the user's typed value withrepo.default_branch. Guarded with auseRefso init runs exactly once per modal-open / mount session.Same anti-pattern as PR #1001 (session list filters being reset by WebSocket events), applied to two more surfaces.
What you'll see fixed
.agor.ymlparser finds it).mainif the daemon emits any unrelated repo patch event before you click Create.Test plan
pnpm -w testclean across@agor/core(2152),@agor/executor(344),agor-ui(109)packages/core/src/git/index.test.ts—cloneRepo:options.branchis setoptions.branchis not setoptions.branchdoes not exist on the remotepackages/core/src/api/index.test.ts—createClient/ authentication configuration:setItem(Node 25)setItemis not a functionpackages/executor/src/payload-types.test.ts—GitClonePayloadSchema:default_branchin paramsdefault_branchas optionalpackages/executor/src/commands/index.test.ts—executeCommand git.clone:default_branchin dry-run responseapps/agor-ui/src/components/NewWorktreeModal/NewWorktreeModal.test.tsx(new file):repoByIdMap reference churnapps/agor-ui/src/components/CreateDialog/tabs/WorktreeTab.test.tsx(new file):repoByIdMap reference churnpreset-io/preset-codespaceswithdefault_branch=geido/feat/codespaces-ultra→ DB record has the pin, working tree is on that branch,.agor.ymlparsed, environment variants populated, new worktrees default to that base.Files changed
packages/core/src/api/index.ts(+ test)apps/agor-ui/src/test/setup.tspackages/core/src/git/index.ts(+ test)CloneOptions.branch→git clone --branch <name>;defaultBranchin result reflects the pinpackages/executor/src/payload-types.ts(+ test)GitClonePayloadSchema.params.default_branch?packages/executor/src/commands/git.ts(+ test)branchtocloneRepo; prefer pin when writing repo DB record; echo in dry-runapps/agor-daemon/src/services/repos.tsdefault_branchfrom request body into executor paramsapps/agor-daemon/src/register-routes.tsdefault_branch?to/repos/clonebody typeapps/agor-ui/src/components/NewWorktreeModal/NewWorktreeModal.tsx(+ test)useRefgate for form-init effectapps/agor-ui/src/components/CreateDialog/tabs/WorktreeTab.tsx(+ test)🤖 Generated with Claude Code