Skip to content

docs: correct which sidebar tab field surface.* verbs accept (#12803) - #12872

Closed
aliyansajid wants to merge 1 commit into
manaflow-ai:mainfrom
aliyansajid:fix/12803-custom-sidebar-surface-focus-docs
Closed

aliyansajid wants to merge 1 commit into
manaflow-ai:mainfrom
aliyansajid:fix/12803-custom-sidebar-surface-focus-docs

Conversation

@aliyansajid

@aliyansajid aliyansajid commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #12803

Summary

What changed? docs/custom-sidebars.md documented the wrong field for focusing a tab from a custom sidebar. The "Live data you can bind to" prose said:

Each workspace's tabs[i] carries surfaceId for surface.* verbs (tabs[i].id is the panel behind the tab, not interchangeable).

That is inverted. The same document's own worked examples (the surface.focus calls in the single-column and two-column sidebars, lines 381 and 418) already pass tabs[j].id — so the prose and the examples contradicted each other.

Why? CustomSidebarDataContextBuilder is authoritative:

  • Layout/CustomSidebarDataContextBuilder.swift:93 builds the per-workspace tabs array from workspace.surfaces
  • :202 sets each entry's "id" from surface.panelId
  • :207-210 sets surfaceId as a separate, optional field

Because the v2 API renamed panels to surfaces, surface.focus's surface_id parameter is that panel UUID — i.e. tabs[i].id. This matches the reporter's independent measurement against cmux tree --id-format both.

Passing tabs[i].surfaceId instead fails silently: the Button fires, no error surfaces anywhere, and the sidebar simply never navigates — so it presents as a hit-testing or layout bug rather than a bad argument. That is what makes the wrong doc costly.

agents[j].surfaceId correlating with tabs[k].surfaceId was already correct and is preserved; only the claim about which field surface.* accepts was wrong. The issue notes these two are easy to conflate, so the fix keeps that distinction explicit.

Also fixed beyond the issue: docs/subagents-panel-plan.md:100 (status: first pass shipped) repeats the identical accepted by surface.focus claim in its agent field table. Corrected the same way so the error does not survive in a second place.

Testing

Docs-only change; verified by reading the source of truth rather than by running the app:

  • Traced tabs[i].id to surface.panelId in CustomSidebarDataContextBuilder.swift (lines 93, 202, 207-210) and confirmed surfaceId is a distinct optional field
  • Confirmed the corrected prose now agrees with the two existing surface.focus examples in the same document
  • Grepped docs/ and skills/ for the same claim to make sure no third copy remains:
    grep -rn "surfaceId" docs/ skills/ | grep -iE "focus|surface\.\*|verb" → only the lines changed here
  • Localization audit (required by CLAUDE.md for docs changes): both files are English-only developer docs with no translated counterparts, and none of the changed text appears in Resources/Localizable.xcstrings or web/messages/en.json, so no catalog updates are required

Demo Video

Not applicable — documentation-only change with no UI or behavior impact. The rendered diff is the whole change.

Checklist

  • I tested the change locally — verified against the authoritative Swift source; no build required for a docs-only change
  • I added or updated tests for behavior changes — N/A, no behavior change
  • I updated docs/changelog if needed — this change is the docs correction
  • I requested bot reviews after my latest commit — posting the trigger comment now
  • All code review bot comments are resolved — pending first review pass
  • All human review comments are resolved — pending first review pass

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Corrects the custom sidebar docs to state that surface.* verbs accept tabs[i].id, not tabs[i].surfaceId as previously documented (fixes #12803).

  • The old prose contradicted the doc's own surface.focus examples and the CustomSidebarDataContextBuilder source, where tabs[i].id is set from surface.panelId and surfaceId is a separate optional field.
  • Passing the wrong field fails silently—the button fires but the sidebar never navigates—so this removes a misleading troubleshooting path.
  • The same correction applies to docs/subagents-panel-plan.md; tabs[i].surfaceId remains documented as the correlation key for agents[j].surfaceId.
  • Docs-only change with no runtime or localization impact.

Written for commit 23b0222. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Documentation
    • Clarified the identifiers used to target tabs and agent surfaces.
    • Documented the distinct roles of id, panelId, and surfaceId.
    • Updated guidance for correlating agents with their hosting tabs and using identifiers with surface controls.

…w-ai#12803)

`docs/custom-sidebars.md` stated that a workspace's `tabs[i]` carries
`surfaceId` for `surface.*` verbs, with `tabs[i].id` as "the panel behind
the tab, not interchangeable". That is inverted, and the document's own
worked examples (the `surface.focus` calls in the single-column and
two-column sidebars) already pass `tabs[j].id`.

`CustomSidebarDataContextBuilder` is authoritative: the per-workspace
`tabs` array is built from `workspace.surfaces`, each entry's `id` is set
from `surface.panelId`, and `surfaceId` is a separate optional field. Since
the v2 API renamed panels to surfaces, `surface.focus`'s `surface_id` is
that panel UUID, i.e. `tabs[i].id`.

Passing `tabs[i].surfaceId` instead fails silently: the button fires, no
error surfaces anywhere, and the sidebar simply never navigates, so it
reads as a hit-testing or layout bug.

`agents[j].surfaceId` correlating with `tabs[k].surfaceId` was already
correct and is preserved; only the claim about which field `surface.*`
accepts was wrong. `docs/subagents-panel-plan.md` (status: first pass
shipped) repeated the same claim in its agent field table and is corrected
the same way.

Localization audit: both files are English-only developer docs with no
translated counterparts, and none of the changed text appears in
`Resources/Localizable.xcstrings` or `web/messages/en.json`, so no catalog
updates are required.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@aliyansajid

Copy link
Copy Markdown
Contributor Author

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 17, 2026

Copy link
Copy Markdown

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@aliyansajid cubic can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 339,945 of the 320,000 allowed lines of code this month. Reviews resume on 1 October 2026 (in 14 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9c2ebcc2-5439-41c8-aa89-aeec8e8d046e

📥 Commits

Reviewing files that changed from the base of the PR and between 89668df and 23b0222.

📒 Files selected for processing (2)
  • docs/custom-sidebars.md
  • docs/subagents-panel-plan.md

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The documentation now identifies tabs[i].id as the value accepted by surface.* verbs. It distinguishes this from tabs[i].surfaceId, which correlates tabs with agents. Related panelId and surfaceId descriptions were updated.

Changes

Surface identifier documentation

Layer / File(s) Summary
Correct surface identifier semantics
docs/custom-sidebars.md, docs/subagents-panel-plan.md
The documentation now assigns tabs[i].id to surface.* calls and uses tabs[i].surfaceId to correlate agents with tabs. The panelId and surfaceId descriptions reflect these roles.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other · Severity of issue fixed: Medium

Suggested reviewers: lawrencecchen

Merge Risk: ⚪ Minimal · up to 23b02

No actionable merge-blocking risk remains; the identifier documentation is consistent and no application behavior changed.


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Cmux Full Internationalization ❌ Error docs/custom-sidebars.md is user-facing production documentation. The app exposes it through cmux docs sidebars and links it as the custom-sidebar authoring guide; the file also gives end users the… Route the changed custom-sidebar guidance through the localized documentation source (using next-intl or an equivalent locale-specific Markdown/content source), and provide matching translated entries for every locale in `web/i18n/routing…
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy #12803. docs/custom-sidebars.md now states that tabs[i].id is the value accepted by surface.* verbs and preserves tabs[i].surfaceId for correlation with `agents[j].surfaceI…
Out of Scope Changes check ✅ Passed The pull request changes only the two documentation files named in the issue and clarifies identifier semantics related to the same surface.* and agent-correlation requirements. It changes no applic…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Cmux Swift Actor Isolation ✅ Passed PASS: The pull request changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. The authoritative diff contains no Swift files or production code, so it cannot introduce or worsen Sw…
Cmux Swift Blocking Runtime ✅ Passed PASS. The review-scoped diff changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. Both files are Markdown documentation. No production Swift code or synchronization implementatio…
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. The diff contains documentation updates for surface.*, surfaceId, and surface_id; it does not ch…
Cmux Expensive Synchronous Load ✅ Passed PASS: The reviewed range changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. The diff contains documentation text only and adds or moves no Swift code, agent-history loads, sync…
Cmux Cache Substitution Correctness ✅ Passed PASS: The reviewed range changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. The diff contains Markdown documentation edits only; no Swift, TypeScript, or JavaScript production …
Cmux No Hacky Sleeps ✅ Passed PASS. The reviewed range changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. The patch updates documentation wording about tabs[i].id, surfaceId, and surface.focus; it int…
Cmux Algorithmic Complexity ✅ Passed PASS — The review-scoped diff changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. It contains documentation edits only; it adds no production Swift, TypeScript, JavaScript, shel…
Cmux Swift Concurrency ✅ Passed PASS: The reviewed range changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. It contains no Swift files or runtime code, so it introduces no legacy Swift concurrency pattern cov…
Cmux Swift @Concurrent ✅ Passed PASS. The review-scoped diff changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. Both changes are Markdown documentation edits, with no Swift source, async function, actor isola…
Cmux Swift Package Boundaries ✅ Passed PASS: The review diff changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. It contains no Swift, app-target, or SwiftPM package changes. The package-boundary failure conditions a…
Cmux Swiftpm Lockfiles ✅ Passed PASS: The reviewed range changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. It contains no Package.swift, Package.resolved, Xcode project/workspace, .gitignore, or workfl…
Cmux Swift Logging ✅ Passed PASS. The review-scoped diff changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. It adds or changes no Swift logging, stdout/stderr diagnostics, print, debugPrint, dump, `…
Cmux User-Facing Error Privacy ✅ Passed PASS: The reviewed range changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. The additions are API documentation about surface.*, tabs, and agent identifiers; they do not ad…
Cmux Swiftui State Layout ✅ Passed PASS. The review-scoped diff changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. It updates documentation prose and adds no SwiftUI implementation, state, geometry measurement, …
Cmux Architecture Rethink ✅ Passed PASS: The reviewed range changes only two Markdown documentation files (docs/custom-sidebars.md and docs/subagents-panel-plan.md); it changes no Swift files or application behavior. Therefore it d…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The pull request changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. No Swift files or window declarations changed. Therefore, it does not introduce or materially change a…
Cmux Source Artifacts ✅ Passed The PR changes only two existing Markdown documentation files: docs/custom-sidebars.md and docs/subagents-panel-plan.md. The patch contains hand-written documentation corrections about surface.*…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The pull-request range changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. It adds no Swift file under a production Sources/ path, no test-build guard, and no test/debug seam.…
Cmux No Ambient Global State ✅ Passed The pull-request range changes only docs/custom-sidebars.md and docs/subagents-panel-plan.md. The patch contains documentation text changes only; it adds no production Swift declarations, globals,…
Title check ✅ Passed The title clearly identifies the primary change: correcting which sidebar tab field the surface.* verbs accept.
Description check ✅ Passed The description explains what changed, why it changed, how it was verified, and why no demo video or behavior tests apply. It also documents the related correction in the second file and preserves the…
Full details: Cmux Full Internationalization

Explanation

docs/custom-sidebars.md is user-facing production documentation. The app exposes it through cmux docs sidebars and links it as the custom-sidebar authoring guide; the file also gives end users the authoring contract. The PR materially changes its rendered Markdown prose, but adds no locale-specific source or entries. The repository has 20 supported web locales in web/i18n/routing.ts. docs/subagents-panel-plan.md is an internal planning document, but that does not remove the violation in the public custom-sidebar guide.

Resolution

Route the changed custom-sidebar guidance through the localized documentation source (using next-intl or an equivalent locale-specific Markdown/content source), and provide matching translated entries for every locale in web/i18n/routing.ts: en, ja, zh-CN, zh-TW, ko, de, es, fr, it, da, pl, ru, bs, ar, no, pt-BR, th, tr, km, and uk. Keep the public docs URL and CLI output selecting the locale-specific content. Alternatively, do not ship the changed prose as user-facing rendered Markdown.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

@aliyansajid I will review pull request #12872.

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@aliyansajid

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document v2.2 and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Sep 17, 2026
@aliyansajid

Copy link
Copy Markdown
Contributor Author

Thanks for the review. On the Cmux Full Internationalization pre-merge check, I believe this is a false positive for this change, and I'd rather explain than paper over it.

The check is right that docs/custom-sidebars.md is user-facing — CLI/CMUXCLI+DocsSettings.swift:136 ships it as the "custom sidebar authoring guide" raw URL. But the prescribed remedy doesn't match how this document is wired today:

  1. The localized docs site doesn't source this file. cmux.com/docs is served from TSX pages under web/app/[locale]/(landing)/docs/, which never consume repo-root docs/*.md. There is no custom-sidebars route directory there, and web/ contains no reference to custom-sidebars at all. The CLI's webURL for this topic points at a page that has no route today; the resource it actually ships is the single English raw file.

  2. English-only is the established posture for docs/. 56 of the 58 files in docs/ have no translated variant; cloud-image-paste.md is the lone exception. This PR doesn't change that posture — it corrects two wrong sentences inside an already-English-only file.

  3. Following the suggestion would mean building a new feature. Creating a localized web docs route for custom sidebars plus translated entries for every locale in web/i18n/routing is a substantial piece of work, and it would contradict this review's own passing Out of Scope Changes check for a fix whose entire purpose is correcting an inverted field name.

Per CLAUDE.md, the requirement for a docs change is a localization audit, which is in the PR description: neither changed file has a translated counterpart, and none of the changed text appears in Resources/Localizable.xcstrings or web/messages/en.json, so there are no catalog entries to update.

Happy to be corrected if the intent is that docs/ should become fully localized — but that seems like it belongs in its own issue rather than as a gate on a two-line correctness fix. Gating it here would mean the wrong documentation stays published in the meantime.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Thanks @aliyansajid! #14284 changed surfaceId to be the focusable id (the projected pane for remote tmux tabs), and the docs now say surface.* takes surfaceId. Switching them back to tabs[i].id would break remote tmux tabs, so I'm closing this as superseded. I'll fix the one leftover t.id example on main and close #12803 with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: custom-sidebars says surface.focus takes tabs[].surfaceId, but it takes tabs[].id (fails silently)

2 participants