Commit 45cec9a
fix(repos,ui): pin user-supplied default_branch end-to-end + don't reset typed sourceBranch on WebSocket events (#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 #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 #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>1 parent 3b7fba1 commit 45cec9a
17 files changed
Lines changed: 691 additions & 51 deletions
File tree
- apps
- agor-daemon/src
- services
- agor-ui/src
- components
- CreateDialog/tabs
- NewWorktreeModal
- SettingsModal
- test
- packages
- core/src
- api
- git
- executor/src
- commands
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2092 | 2092 | | |
2093 | 2093 | | |
2094 | 2094 | | |
2095 | | - | |
| 2095 | + | |
2096 | 2096 | | |
2097 | 2097 | | |
2098 | 2098 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
144 | 144 | | |
145 | 145 | | |
146 | 146 | | |
147 | | - | |
| 147 | + | |
148 | 148 | | |
149 | 149 | | |
150 | 150 | | |
| |||
200 | 200 | | |
201 | 201 | | |
202 | 202 | | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
203 | 207 | | |
204 | 208 | | |
205 | 209 | | |
| |||
217 | 221 | | |
218 | 222 | | |
219 | 223 | | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
220 | 234 | | |
221 | 235 | | |
222 | 236 | | |
223 | | - | |
| 237 | + | |
224 | 238 | | |
225 | 239 | | |
226 | 240 | | |
| |||
Lines changed: 64 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
Lines changed: 11 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
50 | 56 | | |
51 | | - | |
| 57 | + | |
52 | 58 | | |
53 | 59 | | |
54 | 60 | | |
| 61 | + | |
55 | 62 | | |
56 | 63 | | |
57 | 64 | | |
58 | 65 | | |
59 | 66 | | |
60 | 67 | | |
61 | 68 | | |
| 69 | + | |
62 | 70 | | |
63 | 71 | | |
64 | 72 | | |
| |||
Lines changed: 113 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
Lines changed: 15 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
46 | | - | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
47 | 53 | | |
48 | | - | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
49 | 59 | | |
50 | 60 | | |
51 | 61 | | |
52 | 62 | | |
53 | 63 | | |
| 64 | + | |
54 | 65 | | |
55 | 66 | | |
56 | 67 | | |
| |||
60 | 71 | | |
61 | 72 | | |
62 | 73 | | |
| 74 | + | |
63 | 75 | | |
64 | 76 | | |
65 | 77 | | |
| |||
Lines changed: 78 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
0 commit comments