fix(ui): prevent session list from resetting filters on WebSocket events#1001
Draft
Antonio-RiveroMartnez wants to merge 2 commits into
Draft
fix(ui): prevent session list from resetting filters on WebSocket events#1001Antonio-RiveroMartnez wants to merge 2 commits into
Antonio-RiveroMartnez wants to merge 2 commits into
Conversation
The reset useEffect had `initialActiveSessions` (a .filter() result) in its dependency array. Since .filter() always produces a new array reference, every WebSocket session event would trigger a full state reset — clearing the archive toggle, wiping column filters, and re-fetching all data. Changed the dependency to `worktree.worktree_id` so the reset only fires when switching worktrees. WebSocket handlers already keep the list current incrementally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 tasks
mistercrunch
pushed a commit
that referenced
this pull request
May 9, 2026
…set 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>
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
1 participant
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
useEffectdependency that caused session list to reset filters on every WebSocket eventinitialActiveSessions(new array ref on every render) toworktree.worktree_idso reset only fires when switching worktreesTest plan
🤖 Generated with Claude Code