Skip to content

fix(web): folded second mates stay folded after a trip through Settings - #167

Merged
ImZoomBoy merged 3 commits into
mainfrom
fix/project-icon-colour-fleet-tree
Sep 25, 2026
Merged

ImZoomBoy merged 3 commits into
mainfrom
fix/project-icon-colour-fleet-tree

Conversation

@ImZoomBoy

@ImZoomBoy ImZoomBoy commented Sep 25, 2026 •

Copy link
Copy Markdown
Owner

After a project icon edit, the firstmate second mate's branch in the fleet tree opened by itself and filled the sidebar with its workers. That pushed sheppi out of view, so it looked like sheppi had vanished.

Cause

The fleet tree kept its folds in useState inside Sidebar.tsx. A project icon is edited in Settings. On any /settings route, AppSidebarLayout.tsx swaps the thread sidebar out for the settings nav, so Sidebar unmounts. Pressing Back mounts a fresh Sidebar with no folds, and every second mate opens again.

Evidence. I ran a production web build of 0.0.43-ap.7 (the installed version) from the server, against a copy of the user's database. I folded the agos, firstmate and t3code second mates, opened sheppi's second mate, changed sheppi's icon colour in Settings, and pressed Back. The sidebar went from 13 rows to 55. sheppi's second mate went from visible at y=419 to off screen at y=2328. Every fold arrow read "Hide workers" again.

Fix

Second mate folds are now a saved sidebar setting, next to project folds.

  • uiStateStore.ts gains fleetRepoExpandedById beside projectExpandedById, keyed by fleet repository. A repository with no entry shows its workers. It loads and saves with the other sidebar settings, and has a setFleetRepoExpanded action.
  • Sidebar.tsx builds the fleet rows in an exported useFleetTreeRows hook. The hook reads the folds from uiStateStore, so they survive Settings, any project edit, and a restart, the same as project folds.
  • The scroll position of the thread list is unchanged.

Downgrade

The saved settings gain one optional key, fleetRepoExpandedById. An older build ignores it when it loads. It drops the key the next time it saves, so after a downgrade every second mate shows its workers again. Nothing else in the saved settings changes, and nothing is read wrongly.

Colour update: did not reproduce

Expected: the second mate, its workers and every tag for the project take the new colour at once. They already do on main. I tried:

  • Dev build and production build, both against the data copy.
  • One tab (edit in Settings, press Back), and two tabs (the sidebar stays open in one while the other edits the icon).
  • Automatic to green folder-code, green to blue, blue to green, green to orange, and orange to blue.

Each time, the second mate's sailboat and "Second mate" word and the worker's pickaxe and "Worker" word had the new computed colour on the next check. For example, green was oklch(0.627 0.194 149.214) and blue was oklch(0.546 0.245 262.881). The row's project icon changed too. No colour code is changed.

Tests

Sidebar.fleetTree.test.tsx renders useFleetTreeRows from Sidebar.tsx, the code the sidebar runs. It uses two second mates, firstmate with two workers and sheppi with one.

  • Folds. It folds firstmate, unmounts the sidebar, changes sheppi's icon colour, and mounts it again. firstmate must still be folded.
  • Colour. With the sidebar mounted, it swaps in a project list where sheppi's icon is blue. The second mate and its worker must both carry the blue icon.

Each test resets the shared store first with the new resetUiStateForTests(). uiStateStore.test.ts gains a test that a fold survives a save and reload, and that an old save with no folds loads as none.

How each test fails

Folds. I put main's fold back into Sidebar.tsx: useState inside the hook in place of the uiStateStore read. The fold test failed:

     × keeps a folded second mate folded through Settings and a project icon change 19ms
AssertionError: expected [ 'firstmate-mate', …(4) ] to deeply equal [ 'firstmate-mate', …(2) ]
      Tests  1 failed | 1 passed (2)

The hook does not exist on main, so this is main's fold code in the same code path.

Colour. I dropped projectByKey from the hook's memo dependencies, so rows keep the old project. The colour test failed:

     × gives a second mate and its workers the project's new colour at once 7ms
AssertionError: expected { kind: 'lucide', …(2) } to match object { color: 'blue' }
      Tests  1 failed | 1 passed (2)

With the change as pushed, both pass.

Screenshots

They show the user's real thread titles, so they are kept locally and not attached.

  • Before, folded: First Mate, three folded second mates, sheppi's second mate and worker in view.
  • After Back on ap.7: agos's workers fill the view, then firstmate's settled workers. sheppi is far below.
  • After Back with the first version of this fix: First Mate and four second mates, agos and sheppi still folded, sheppi in blue. That build kept folds for the session only. The saved version was not rechecked in a build.

fork-features.json

The fm-fleet-sidebar-tree entry drops fleetFolds.ts and its test. It lists uiStateStore.ts, uiStateStore.test.ts and Sidebar.fleetTree.test.tsx. The keep line names useFleetTreeRows in Sidebar.tsx. In uiStateStore.ts it names the fleetRepoExpandedById field with its load and save lines, setFleetRepoExpanded and resetUiStateForTests. In uiStateStore.test.ts it names the fleetRepoExpandedById lines and the folded second mates test. The description says the fold is saved like a project fold.

Checks

cd apps/web && vp run typecheck - exit 0, no errors.

vp lint --report-unused-disable-directives apps/web/src/components/Sidebar.tsx apps/web/src/components/Sidebar.fleetTree.test.tsx apps/web/src/uiStateStore.ts apps/web/src/uiStateStore.test.ts - exit 0, 26 warnings, all in Sidebar.tsx. main shows the same 26 for Sidebar.tsx.

vp fmt --check apps/web/src/components/Sidebar.tsx apps/web/src/components/Sidebar.fleetTree.test.tsx apps/web/src/uiStateStore.ts apps/web/src/uiStateStore.test.ts fork-features.json:

All matched files use the correct format.
Finished in 212ms on 5 files using 24 threads.

cd apps/web && vp test run --passWithNoTests --project unit:

 Test Files  411 passed (411)
      Tests  5475 passed (5475)

node scripts/check-fork-features.ts - exit 1:

Fork features: 27 entries, every file and test present.
 Test Files  1 failed | 21 passed (22)
      Tests  11 failed | 451 passed (462)

All 11 failures are in scripts/build-desktop-artifact.test.ts, which this change does not touch. The same file fails the same way on main (c0ed528ff6) on this Windows machine:

 Test Files  1 failed (1)
      Tests  11 failed | 60 passed (71)

Surfaces

Web and desktop share this sidebar. The mobile sidebar has no fleet tree.

The fleet tree kept its folds in the Sidebar component. Settings swaps that
component out, so pressing Back reopened every second mate. After a project
icon edit, the firstmate second mate's settled workers filled the list and
pushed sheppi out of view.

The folds now live in a small store for the app session, so they survive
Settings and project edits. They are still not saved across restarts.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M labels Sep 25, 2026
@ImZoomBoy

Copy link
Copy Markdown
Owner Author

Code review, two axes. Each finding has a confidence and an estimated severity.

Standards

Standards review: PR 167

The PR breaks no documented standard. I found six judgement calls. Four are in the test file and two are in the new store.

The commit title follows the conventional format. The PR has one concern. The fork-features.json entry covers the new file and its test, as docs/agents/upstream-conflict-resolution.md requires. The user docs never mention folding, so this fix needs no docs change.

apps/web/src/components/sidebar/fleetFolds.test.tsx

  1. The colour step can never fail (AGENTS.md "Verifying"). This is a judgement call against the rule "test meaningful logic or observable behavior, do not ... mirror the implementation". The color prop never reaches the store, so this assertion passes whatever the code does:
    act(() => renderer!.update(<SecondMateFolds ... color="blue" />)); expect(labels()).toEqual(["folded", "open"]);
    • Confidence: medium. Severity: low.
  2. The test goes through React to test a store. Other store tests in this repo (for example browserFaviconStore.test.ts) call the store directly. useFleetFolds.getState().toggle(...) would test the same thing without the SecondMateFolds stand-in component.
    • Confidence: medium. Severity: low.
  3. The tests share store state. The comment "Each test uses its own repository names: the folds live for the app session" means the test order matters. The repo's other stores reset between tests with a reset...ForTests() helper, such as resetBrowserHistoryForTests. This PR has no reset.
    • Confidence: high. Severity: low.
  4. The test file uses ! non-null assertions in several places, such as renderer!.root and [index]!.props. This leans against AGENTS.md "Taste" (types should be inferred and sound).
    • Confidence: low. Severity: low.

apps/web/src/components/sidebar/fleetFolds.ts

  1. The store does not follow the repo's store pattern. This is a judgement call. Other stores are named *Store.ts and export use*Store. They declare a named state type and call create<State>()(...) in two steps. This store is named useFleetFolds, uses an inline type, and calls create<{...}>((set) => ...) in one step. The name also hides that this is a global store (Mysterious Name smell).
    • Confidence: medium. Severity: low.
  2. Fold state now lives in two places (Duplicated Code smell). Project folding already lives in uiStateStore.ts as projectExpandedById. That store is saved to disk. Second-mate folding is now a second, separate store that is not saved. A separate file does keep the fork's code clear of upstream conflicts. That is a fair reason, so I only flag it.
    • Confidence: medium. Severity: low.

apps/web/src/components/Sidebar.tsx

  • The new comment explains the old bug more than current use. "which Settings unmounts" describes why the fix exists. AGENTS.md "Taste" says comments should describe how a thing is used.
    • Confidence: low. Severity: low.

fork-features.json

  • No findings. The description, keep, files and test entries all name the new file and its test.

Spec

PR #167 does fix the bug the user clarified. Its test only checks the new store, though, not the sidebar code that uses it. I found 2 medium and 4 low issues. I did not run the tests, because this copy has no dependencies installed.

(a) Missing or partial

  1. The colour requirement has no guard test. Spec: "the second mate row, its workers, and the project's tags use the new colour at once."

    • The worker says it did not reproduce, on dev and production builds and five colour changes. They give the computed colours as evidence.
    • Leaving the colour code alone is reasonable. But the spec asked for a test for each bug, and the colour wiring in Sidebar.tsx stays untested.
    • Confidence high. Severity low.
  2. The test does not really fail on main. Spec: "A test for each bug that fails on current main."

    • It only fails against a useState stand-in for the store, not against main's Sidebar.tsx.
    • fleetFolds.test.tsx renders its own stand-in component. If someone moved Sidebar.tsx back to useState, the test would still pass.
    • The body says openly how the red run was done.
    • Confidence high. Severity medium.
  3. Folds last only for the app session, while project folds survive a restart. Spec (clarified): "fold state must survive icon edits, the Settings round trip, and any other project change."

    • The session-only store meets that literally. Restarts were not asked for.
    • But project folds persist in uiStateStore.ts:23 (projectExpandedById). So after a restart, the second mates spring open again and the two kinds of fold behave differently.
    • Confidence medium. Severity medium.

(b) Scope creep

None. The diff is one new store, two changed lines in Sidebar.tsx, one test and the fork-features.json entry. Spec: "Nothing else about the sidebar changes."

(c) Looks done, may be wrong

  1. Folds are keyed by repository name only, not per environment. Two environments with the same repo name share one fold. Main behaved the same way. Confidence medium. Severity low.

  2. A fold for a repo that no longer exists stays in the set. It is harmless: it is only read through .has(repo), and it re-applies if the repo comes back. The set never shrinks, but it is tiny. Threads and repos appearing or disappearing cannot reset folds, because the store is separate from the component. Confidence high. Severity low.

  3. The module-level store is never reset between tests. The tests avoid clashes by using different repo names. A future test that reuses a name would see leftover state. Confidence high. Severity low.

Other checks

  • Merge with fix(web): parked fleet threads list in Settled and Snoozed, compact rows keep titles #166: it is already on main (c0ed528ff6), so there is nothing left to merge.
  • fork-features: the entry lists fleetFolds.ts and fleetFolds.test.tsx, in its files, keep text, tests and description. Spec: "Update the fork-features.json entry for the fleet tree."
  • Causes: the body names each cause with evidence. The disappearance is Settings unmounting Sidebar, which reopens every folded second mate so sheppi scrolls out of view.
  • Fork features check: it exits 1. The body pastes that output and shows the same 11 failures in scripts/build-desktop-artifact.test.ts on main.

@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −1 B (−0.0%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB −5 B (−0.1%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.4 KiB 6.4 KiB +4 B (+0.1%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.1 KiB 56.1 KiB 0 B (0.0%) 66.4 KiB ✅
Codex Live turn messages 7 7 0 (0.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB +18 B (+0.1%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB +2 B (+0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.4 KiB +16 B (+0.2%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 56.9 KiB 56.9 KiB 0 B (0.0%) 66.4 KiB ✅
Claude Live turn messages 7 7 0 (0.0%) 21 ✅

Baseline: c0ed528 · PR result: 8920f61 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 114.0 KiB
  • Claude decoded thread snapshot: 114.7 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Second mate folds now live in uiStateStore as fleetRepoExpandedById, next to
projectExpandedById, and are saved the same way. They survive Settings,
project edits and a restart, like project folds. The session store
fleetFolds.ts and its test are removed.

Sidebar.tsx builds the fleet rows in useFleetTreeRows. Sidebar.fleetTree.test.tsx
renders that hook: a folded second mate stays folded after a remount and an
icon change, and a second mate and its workers take a project's new colour at
once. uiStateStore gains resetUiStateForTests.
@github-actions github-actions Bot added size:L and removed size:M labels Sep 25, 2026
@ImZoomBoy

Copy link
Copy Markdown
Owner Author

This round, per review finding. Commit 0f4e111301.

Standards

  1. Colour step could never fail - fixed. The new colour test in Sidebar.fleetTree.test.tsx fails when projectByKey is dropped from the row memo (output in the body).
  2. Test went through React to test a store - the store test is gone. The new test renders useFleetTreeRows from Sidebar.tsx on purpose, since it guards the sidebar code path. The saved fold itself is tested directly in uiStateStore.test.ts.
  3. Tests shared store state - fixed. resetUiStateForTests() in uiStateStore.ts runs before each test.
  4. ! assertions in the test - fixed in the new test. The one left is in moved Sidebar.tsx code (row.fold!), as on main.
  5. Store naming did not follow the repo pattern - fixed by removal. Folds now use uiStateStore's existing pattern: a pure setFleetRepoExpanded plus a store action.
  6. Fold state in two places - fixed. Second mate folds sit in uiStateStore beside projectExpandedById.
    Sidebar comment described the old bug - fixed. The hook's comment says what it does.

Spec

  1. No colour guard test - fixed, see Standards 1.
  2. Test did not fail on main - fixed. Putting main's useState fold back into Sidebar.tsx fails the new test (output in the body).
  3. Folds lasted only for the session - fixed. They are saved and survive a restart, like project folds. The body notes what a downgrade does.
  4. Folds keyed by repository only - left. main behaves the same, and project-level keys would change what the user sees.
  5. Folds for gone repositories stay saved - left. Harmless and tiny, and a returning repository keeps its fold. Project folds work the same way.
  6. Store never reset between tests - fixed, see Standards 3.

Only the store action uses it, as with setPullRequestMergeMethod, so the
unused code check no longer flags it.
@ImZoomBoy
ImZoomBoy merged commit 5791bce into main Sep 25, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant