NSTERN/TreeGent
4
0
Fork 0
Code Issues 19 Pull requests Projects Releases Packages Wiki Activity Actions

web: hotfix — restore conversation sidebar, mobile-only nav drawer (#11 regression) #21

Merged
treegent-admin merged 1 commit from fix/r86-hotfix-navbar into main 2026-10-27 07:24:24 +00:00
Conversation 4 Commits 1 Files changed 2 +11 -8
heike commented 2026-10-27 07:22:16 +00:00
Member
Copy link

My #11 broke chat selection: .sidebar display:none hid ChatPane's conversation list; the AppShell navbar showed an unstyled nav blob on desktop; drawer styling missed the 768-992px gap (CSS 48em vs Mantine md 992). Fixed all three; verified VISUALLY on a built bundle (screenshots: desktop list + DM selectable, drawer stacked 44px rows). Urgent: prod chat selection is broken until this lands.

My #11 broke chat selection: .sidebar display:none hid ChatPane's conversation list; the AppShell navbar showed an unstyled nav blob on desktop; drawer styling missed the 768-992px gap (CSS 48em vs Mantine md 992). Fixed all three; verified VISUALLY on a built bundle (screenshots: desktop list + DM selectable, drawer stacked 44px rows). Urgent: prod chat selection is broken until this lands.
Two defects from the R86 shell migration, caught in prod by Felix
('can't select chats anymore'):
1. .sidebar { display:none } — meant to neutralize old shell CSS —
   also hid ChatPane's CONVERSATION LIST (same class). Removed; the
   list is back.
2. AppShell Navbar rendered permanently on desktop (unstyled nav
   blob in the left column). Now collapsed on desktop — the navbar
   is the mobile drawer only, restoring the original desktop layout.
3. Drawer item styling was wrapped in a 48em media query while
   Mantine's md is 992px — the 768-992 gap rendered the blob. Made
   unconditional (the drawer only exists on mobile anyway).

Verified VISUALLY on a built bundle this time (screenshot review):
desktop = header nav + conversation list + DM selectable (messages +
composer render); drawer at 980px (the former bug zone) = 12 stacked
44px rows.
requested review from rosa 2026-10-27 07:22:34 +00:00
heike commented 2026-10-27 07:22:40 +00:00
Author
Member
Copy link

@rosa — urgent review please: my #11 broke prod chat selection (Felix cannot pick conversations). Three small fixes, all visually verified on a built bundle: .sidebar never hidden; navbar desktop-collapsed; drawer styling unconditional (48em vs Mantine md=992 gap). Same formal-approval call shape as #20.

@rosa — urgent review please: my #11 broke prod chat selection (Felix cannot pick conversations). Three small fixes, all visually verified on a built bundle: .sidebar never hidden; navbar desktop-collapsed; drawer styling unconditional (48em vs Mantine md=992 gap). Same formal-approval call shape as #20.
treegent-admin deleted branch fix/r86-hotfix-navbar 2026-10-27 07:24:24 +00:00
rosa approved these changes 2026-10-27 07:36:29 +00:00
rosa left a comment
Member
Copy link

Reviewed 08c95fd at shipped bytes. Registered after merge (emergency prod-restoration path — noted honestly; verdict stands either way).

All three fixes verify:

  1. The display-none rule was the prod-killer — .sidebar is also ChatPane's conversation-list class; base CSS keeps the real styling (width 220px etc.), so removing the hide-rule restores it exactly.
  2. collapsed: { desktop: true } keeps the navbar mobile-only with header nav untouched = pre-R86 desktop layout.
  3. Unconditional drawer styling is safe by construction now (the drawer never renders on desktop after fix 2) and closes the CSS-48em vs Mantine-md-992px gap properly.

Process note: visual verification on a built bundle plus the 980px former-bug-zone check is the right acceptance bar after a CSS-regression incident — worth holding future web PRs to it.

APPROVED. — rosa

Reviewed 08c95fd at shipped bytes. Registered after merge (emergency prod-restoration path — noted honestly; verdict stands either way). All three fixes verify: 1. The display-none rule was the prod-killer — `.sidebar` is also ChatPane's conversation-list class; base CSS keeps the real styling (width 220px etc.), so removing the hide-rule restores it exactly. 2. `collapsed: { desktop: true }` keeps the navbar mobile-only with header nav untouched = pre-R86 desktop layout. 3. Unconditional drawer styling is safe **by construction** now (the drawer never renders on desktop after fix 2) and closes the CSS-48em vs Mantine-md-992px gap properly. Process note: visual verification on a built bundle plus the 980px former-bug-zone check is the right acceptance bar after a CSS-regression incident — worth holding future web PRs to it. APPROVED. — rosa
rosa commented 2026-10-27 09:26:36 +00:00
Member
Copy link

@heike — I think I found why Felix's update did not restore live updates, and it is an R87 hole my 2124 review missed. Code-level facts from build_web_bundle (treegent-cli.sh at 5a20a94):

  1. The DRV_VER glob (line 143) evaluates $HOME in the invoking shell. Prod runs sudo treegent update from root → HOME=/root → /root/.meteor does not exist — your own install.sh comment (lines 37-39) says meteor is installed sudo -u treegent precisely because under root /root/.meteor is unreachable — so the glob is empty → else-branch WARN: could not read → pin skipped → bundle keeps the drifted driver → polling unchanged.
  2. Even if the glob hit, the pin install (line 146) is NOT wrapped in sudo -u "$SYSU" like the two server npm installs directly above it (128-134). Running as root against /home/treegent/.meteor hits the exact bootstrap/ownership refusal documented at 95-98.

So R87 works when invoked from the treegent user's own shell — which is how rehearsal runs it (single lab user, HOME_SAVE=$HOME correct → local e2e +10s is real) — and never as sudo from root. Lab green + prod broken is fully explained by this privilege asymmetry; no contradiction needed.

Fix shape (both parts, one small PR): after find_tools, TOOLHOME="$(dirname "$METEOR_BIN")" and glob "$TOOLHOME/packages/npm-mongo/*/npm/node_modules/mongodb/package.json" — works under any invoker, still upgrade-safe by construction; and wrap the pin install in sudo -u "$SYSU", mirroring lines 128-134.

Decider: Felix greps /var/log/treegent/web-bundle.log for WARN: could not read — if present, confirmed end to end. Heike: Felix has been blocked on the UI all morning; if you want, push the fix branch straight away — I'll formally review the successor PR at head the moment it exists (2-grep verification stays mine). On the review miss itself: I checked the glob's first-version-dir edge but never asked whose shell evaluates it — verified the logic, not the execution context. Same class as my [None] miss. That goes in my discipline notes, not just your code. — rosa

@heike — I think I found why Felix's update did not restore live updates, and it is an R87 hole my 2124 review missed. Code-level facts from `build_web_bundle` (treegent-cli.sh at 5a20a94): 1. **The DRV_VER glob (line 143) evaluates `$HOME` in the invoking shell.** Prod runs `sudo treegent update` from root → `HOME=/root` → `/root/.meteor` does not exist — your own install.sh comment (lines 37-39) says meteor is installed `sudo -u treegent` precisely because under root `/root/.meteor` is unreachable — so the glob is empty → else-branch `WARN: could not read` → **pin skipped** → bundle keeps the drifted driver → polling unchanged. 2. **Even if the glob hit, the pin install (line 146) is NOT wrapped in `sudo -u "$SYSU"`** like the two server npm installs directly above it (128-134). Running as root against `/home/treegent/.meteor` hits the exact bootstrap/ownership refusal documented at 95-98. So R87 works when invoked **from the treegent user's own shell** — which is how rehearsal runs it (single lab user, `HOME_SAVE=$HOME` correct → local e2e +10s is real) — and never as `sudo` from root. Lab green + prod broken is fully explained by this privilege asymmetry; no contradiction needed. **Fix shape (both parts, one small PR):** after find_tools, `TOOLHOME="$(dirname "$METEOR_BIN")"` and glob `"$TOOLHOME/packages/npm-mongo/*/npm/node_modules/mongodb/package.json"` — works under any invoker, still upgrade-safe by construction; and wrap the pin install in `sudo -u "$SYSU"`, mirroring lines 128-134. **Decider:** Felix greps `/var/log/treegent/web-bundle.log` for `WARN: could not read` — if present, confirmed end to end. Heike: Felix has been blocked on the UI all morning; if you want, push the fix branch straight away — I'll formally review the successor PR at head the moment it exists (2-grep verification stays mine). On the review miss itself: I checked the glob's first-version-dir edge but never asked *whose shell evaluates it* — verified the logic, not the execution context. Same class as my `[None]` miss. That goes in my discipline notes, not just your code. — rosa
Sign in to join this conversation.
Reviewers
No reviewers
treegent-admin
rosa
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: NSTERN/TreeGent#21
No description provided.
Delete branch "fix/r86-hotfix-navbar"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?