Skip to content

fix(macos): allow browser sidebar resizing with few tabs - #137992

Merged
steipete merged 3 commits into
mainfrom
codex/fix-macos-browser-divider
Sep 4, 2026
Merged

fix(macos): allow browser sidebar resizing with few tabs#137992
steipete merged 3 commits into
mainfrom
codex/fix-macos-browser-divider

Conversation

@steipete

@steipete steipete commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Related: #136157, #136168

What Problem This Solves

Fixes an issue where users dragging the macOS dashboard's browser divider could not resize the sidebar when only one or two tabs were open. The browser could stay at its 320-point minimum even after the earlier dashboard minimum-width fix.

Why This Change Was Made

The tab strip constrained its scroll document to be at least as wide as the viewport, while AppKit's horizontal stack and capped tab widths constrained that same document in the opposite direction. The hidden single-tab viewport also forced its retained contents to zero height. Letting the document keep its natural width and canonical 30-point height removes those conflicting constraints at their owner.

The existing native divider and width persistence remain the canonical path. The smoke test now lays out one and two tabs, moves the real native divider through AppKit's constraint-aware positioning API, and checks actual pane widths plus saved-preference restoration. This replaces callback-only proof and removes its unused test hooks and stored split-view reference.

User Impact

Users can widen or narrow the browser with one or multiple tabs, including beyond half the dashboard window, and retain their chosen width after closing and reopening it. Overflowing tab strips still scroll. There are no new settings, dependencies, or storage changes.

Evidence

  • Signed isolated Swift Testing reproduction using the actual browser view, tab bar, and native split controller: before the fix, requested browser widths of 600 and 450 points stayed at 320 with one tab and 370 with two tabs; after the fix both widths matched exactly for one, two, and eight tabs. Eight-tab overflow remained scrollable. The original fails with four recorded assertions and exit 1; the fixed fixture reports individual, suite, and run success with exit 0.
  • Physical pointer dragging reproduced the stuck divider before the fix and moved it after the fix in the same synthetic native fixture.
  • Rebuilt and Developer-ID-signed the complete macOS app, installed it, and verified physical dragging in both directions in the actual dashboard. Closing and reopening the browser restored its dragged width. Private dashboard captures are intentionally excluded from this public PR.
  • swiftformat --lint with config/swiftformat, Swift parser validation, and git diff --check passed. The full app package build and strict code-signature verification passed.
  • The strengthened DashboardWindowSmokeTests regression runs in the disposable macOS CI lane; the repository's native test runner forbids executing the full application suite on an operator desktop. The isolated signed reproduction does not load OpenClaw startup, Gateway state, or saved credentials.
  • Final native CI on c9bc811ab83: the sidebar regression, all four Config recovery cases, and the full default app suite explicitly passed (2,082 tests across 218 suites). The shared kit passed 1,627 tests across 124 suites; named-profile and isolated health-fixture checks passed. The release build and shared-kit code scans also passed.
  • Independent Codex review: no actionable P0–P2 findings.

Automated layout checks use NSSplitView.setPosition, which applies the same constraints as user dragging. Actual drag persistence is covered by the separate installed-app proof. Synthetic nested mouse tracking inside Swift Testing was removed after a green preliminary job lacked test-completion evidence; the revised isolated fixture explicitly verifies completion.

Native CI also exposed an existing Config-settings test readiness race: after releasing a suspended Gateway acquisition, the test could press Reload while SwiftUI still rendered its loading view. The test now uses its existing bounded rendering helper to wait for the enabled retry control. It retains the stale-error assertions and real accessibility press; no timeout or app behavior changes. The sidebar regression and complete DashboardWindowSmokeTests suite already passed in that run.

Runtime production delta: +4/-6 (net -2). Removed DEBUG test support: -10. Test-file delta: +35/-5 (net +30), giving combined test/support growth of 20 lines and a total PR delta of +18. The two production source files shrink by 12 lines overall.

Before/after screenshots below show the same pointer drag from divider position 779 to 550 in an 1100-point synthetic AppKit window. Before: the browser stays at 320 points. After: the browser grows to 549 points. These captures contain only the synthetic fixture and about:blank.

Before: divider stays stuck after dragging

After: the same drag widens the browser

Regression provenance: the parent-relative patch for 477b2860d5e (#103438) introduced these tab-document constraints; they remain unchanged on the pre-fix head. Git attributes that commit to Peter Steinberger, with GitHub as committer; the human merger was not established by the inspected Git evidence. The hidden-single-tab behavior arrived separately in #107798.

Keep the tab strip document independent of its clipping viewport so capped tab widths and hidden tabs cannot pin the native divider. Exercise AppKit drag tracking and saved widths in the smoke test, replacing callback-only test hooks.
@steipete steipete self-assigned this Sep 4, 2026
@clawsweeper

clawsweeper Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@openclaw-barnacle openclaw-barnacle Bot added app: macos App: macos size: S maintainer Maintainer-authored PR labels Sep 4, 2026
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 4, 2026
@clawsweeper

clawsweeper Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Codex review: blocked before merge. Reviewed September 4, 2026, 5:30 AM ET / 09:30 UTC.

ClawSweeper review

What this changes

The PR removes conflicting macOS tab-strip constraints so the dashboard link-browser sidebar can resize with one or two tabs, and replaces callback-only checks with native split-view geometry coverage.

Merge readiness

Blocked before merge - 3 items remain

No introduced correctness finding. This PR remains necessary because current main retains the conflicting tab-strip constraints; the protected maintainer path requires a maintainer proof-sufficiency decision.

Priority: P2
Reviewed head: c9bc811ab83b445296e65c4e7d3827b6c3b5956c
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) Focused owner-bound repair, relevant regression coverage, and exact-head native CI support a normal good PR; the prepared visual bundle mismatch remains a reviewer limitation.
Proof confidence 🐚 platinum hermit (4/6) Not applicable: Repository State identifies the author as a MEMBER, so the external-contributor proof gate does not apply. Exact-head native CI and the owner-bound AppKit geometry test support the change; the prepared bundle contained only older related tab screenshots, not the current divider captures.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: Repository State identifies the author as a MEMBER, so the external-contributor proof gate does not apply. Exact-head native CI and the owner-bound AppKit geometry test support the change; the prepared bundle contained only older related tab screenshots, not the current divider captures.
Evidence reviewed 9 items Constraint owner: The introduced repair removes the document-view constraints that bind the horizontal tab stack to its clipping viewport and retains only leading/top plus the canonical 30-point tab-strip height.
Caller and persistence boundary: Opening the sidebar restores width through the real split view, while the existing divider mouse handler remains the sole production path that writes the existing preference after an actual movement.
Sibling visibility invariant: The browser view hides the tab bar and sets its outer height to zero for one tab, then restores 30 points for multiple tabs; the repaired document itself has 24-point items and 3-point vertical insets, matching the fixed 30-point height.
Findings None None.
Security None None.

How this fits together

The macOS dashboard opens web links in an embedded sidebar beside the main dashboard. The tab strip supplies layout constraints to the native split view, which controls user resizing and saved browser-width restoration.

flowchart LR
  A[Dashboard link] --> B[Embedded browser sidebar]
  B --> C[Tab strip document layout]
  C --> D[Native split view]
  D --> E[Resizable browser width]
  E --> F[Saved width on reopen]
Loading

Decision needed

Question Recommendation
Should the maintainer accept the exact-head native CI and source review despite the prepared media bundle not containing this PR’s fresh divider before/after captures? Accept current evidence: Accept the focused owner repair on the exact-head native CI, source review, and reported installed-app verification.

Why: The author is identified as a MEMBER in Repository State and the PR has a protected maintainer label; the missing prepared attachments are a reviewer-side evidence gap rather than an introduced code defect.

Before merge

  • Resolve merge risk (P1) - The prepared media bundle omitted this PR’s current divider before/after attachments, so the visual claim was not independently checked here; exact-head native CI and source-level regression coverage remain available evidence.
  • Complete next step (P2) - A macOS maintainer must decide whether the exact-head CI and source review are sufficient despite the prepared media bundle omitting the current divider captures.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.
Agent review details

Security

None.

PR surface

Other +18. Total +18 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 4 39 21 +18
Total 4 39 21 +18

Review metrics

Metric Value Why it matters
Production versus test delta runtime production +4/-6 (net -2); tests/test support +35/-5 (net +30) The repair reduces production surface while adding owner-bound layout coverage and removing DEBUG-only callback support.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Land the owner-bound tab-document constraint repair after a macOS maintainer accepts the available native CI and source evidence, keeping the existing divider persistence path unchanged.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Land the owner-bound tab-document constraint repair after a macOS maintainer accepts the available native CI and source evidence, keeping the existing divider persistence path unchanged.

Do we have a high-confidence way to reproduce the issue?

Yes—source-reproducible: the retained current-main constraint set and the native fixture’s one- and two-tab split positioning define a focused path, although AppKit cannot be executed in this Linux review environment.

Is this the best way to solve the issue?

Yes. The tab strip owns the conflicting document constraints; changing divider persistence or adding a downstream clamp would leave that owner conflict intact.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e0c230789b33.

Labels

Label changes:

  • remove proof: sufficient: Current real behavior proof status is not_applicable, not sufficient.

Label justifications:

  • P2: This is a bounded macOS sidebar-resize defect without data, security, message-delivery, or availability impact.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: Repository State identifies the author as a MEMBER, so the external-contributor proof gate does not apply. Exact-head native CI and the owner-bound AppKit geometry test support the change; the prepared bundle contained only older related tab screenshots, not the current divider captures.

Evidence

What I checked:

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)
  • reviewed 2026-09-04T06:54:46.983Z sha 17d733b :: needs maintainer review before merge. :: none
  • reviewed 2026-09-04T07:53:20.845Z sha eaf1354 :: blocked before merge. :: none

@steipete

steipete commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer review completed for 17d733b297cbb9128cb538b210a3864bc22aa423: no accepted findings or outstanding before-merge/rank-up actions.

One evidence clarification: the two screenshots attached in this PR are fresh captures of this divider reproduction, not predecessor tab-feature media. They use the same signed synthetic AppKit fixture with the repository's actual browser view, changing only the pre-fix versus fixed tab-bar source. The same physical drag from position 779 to 550 stays at 779 before and reaches 550 after. Full installed-app drag and close/reopen persistence were also verified locally; private dashboard screenshots are not uploaded.

CI attempt 1 completed its macOS support tests, security and native-localization checks, but its two Swift jobs never received Blacksmith runners. After verifying only queued jobs remained, those pending jobs were retried through the existing GitHub-hosted macOS fallback. Attempt 2 keeps the same source SHA and is now running the native checks. The shared-kit scan received the same targeted recovery.

Use native split-view positioning after tabs are laid out, and test saved-width restoration separately. This avoids entering AppKit mouse tracking inside Swift Testing, which suppressed terminal test reporting in the signed reproduction. Physical drag and persistence were verified in the installed app.
@steipete

steipete commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Updated to eaf1354eb03b00e522e681eb4aa487f97ae5f1a3 after inspecting the preliminary green CI job's detailed log. That job did not establish terminal completion of the new synthetic mouse-tracking test, so it is not counted as final regression proof.

A signed isolated Swift Testing fixture reproduced the missing completion report when entering AppKit's nested mouse-tracking loop. The regression now uses NSSplitView.setPosition after the real tabs are laid out; AppKit documents that this applies the same constraints as a user drag. It checks actual geometry, then seeds a chosen width for the existing restoration checks. Physical dragging and actual preference saving remain separately verified in the installed app and the attached native before/after captures.

The revised isolated test fails with four recorded width assertions on the original tab bar and reports an explicit successful test/suite/run completion on the fixed tab bar, including one/two/eight-tab resizing and overflow. Production code is unchanged by this follow-up. Independent review is clean; final exact-head CI is running again, and the named test's terminal result will be verified before merge.

@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Sep 4, 2026
@steipete

steipete commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the latest review's environment-dependent evidence gap for eaf1354eb03b00e522e681eb4aa487f97ae5f1a3.

I directly inspected Apple's AppKit SDK NSSplitView.h:79–81 and the published positioning contract. Both describe positioning as a simulated user drag subject to the view/delegate constraints. The Markdown documentation is accessible without JavaScript. The test lays out the real tab contents before positioning and asserts the resulting pane frames; it explicitly seeds the preference for restoration checks and does not claim to test drag-triggered saving.

I also re-inspected the full, freshly captured synthetic before and after images. They show this divider scenario: the same physical drag leaves the browser at 320 points before the change and widens it to 549 afterward. Physical dragging in both directions and actual preference saving were separately verified in the rebuilt, Developer-ID-signed installed app.

Maintainer decision: retain the tab-document owner repair and the existing native divider/persistence flow. The review reports no introduced defect, and its unavailable-environment concern is covered by direct local SDK/visual inspection plus the independent final-patch review. No code change is needed for that concern. Final CI is still running; merge remains gated on the named resize test and full app-suite terminal results.

@clawsweeper clawsweeper Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Sep 4, 2026
@steipete
steipete merged commit 2800b64 into main Sep 4, 2026
103 of 111 checks passed
@steipete
steipete deleted the codex/fix-macos-browser-divider branch September 4, 2026 09:32
@steipete

steipete commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Sep 5, 2026
…7992)

* fix(macos): allow browser sidebar resizing with few tabs

Keep the tab strip document independent of its clipping viewport so capped tab widths and hidden tabs cannot pin the native divider. Exercise AppKit drag tracking and saved widths in the smoke test, replacing callback-only test hooks.

* test(macos): verify divider constraints without nested tracking

Use native split-view positioning after tabs are laid out, and test saved-width restoration separately. This avoids entering AppKit mouse tracking inside Swift Testing, which suppressed terminal test reporting in the signed reproduction. Physical drag and persistence were verified in the installed app.

* test(macos): wait for Config retry controls to render
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: macos App: macos maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant