Skip to content

refactor(desktop): register Workbar surface descriptors - #4747

Closed
testikun wants to merge 1 commit into
apache:mainfrom
testikun:codex/issue-4744-workbar-descriptors
Closed

refactor(desktop): register Workbar surface descriptors#4747
testikun wants to merge 1 commit into
apache:mainfrom
testikun:codex/issue-4744-workbar-descriptors

Conversation

@testikun

@testikun testikun commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Register every first-party Workbar tab kind in one exhaustive internal surface descriptor map.
  • Route localized labels, icons, launcher rows, and panel rendering through the descriptors while retaining the existing topology registry as the persistence, singleton, shortcut, and ordering authority.
  • Preserve lazy panel loading, tab identity and placement behavior, preview/pin/focus/close/reorder actions, and Side Conversation/Terminal resource state.
  • Add a structural completeness ratchet plus a real Electron regression for singleton and dynamic tab rendering.

Fixes #4744

Verification

  • node --test apps/desktop/dist/main/__tests__/workbar-boundary.test.js apps/desktop/dist/main/__tests__/workbar-model.test.js — 14/14 passed after the base sync.
  • npx playwright test --config e2e/playwright.config.ts e2e/session-workbar.spec.ts --grep "registered Workbar descriptors" — 1/1 passed in a real Electron fixture after the base sync.
  • npm --workspace @maka/desktop run typecheck — passed after the base sync.
  • npm --workspace @maka/desktop run check:architecture -- --base apache/main — 71/71 checker fixtures passed; renderer architecture check passed against apache/main.
  • npm --workspace @maka/desktop run build — passed, including renderer entry and third-party notice checks.
  • npm run lint and npm run format:check — passed across the repository.
  • git diff --check — passed.

Built and launched current head b67c360ce in a visible local Electron App and ran the registered-descriptor journey: the launcher exposed every registered first-party tool, singleton Todo stayed single, and Terminal and Side Conversation opened as distinct dynamic tabs.

Real App screenshot

This direct capture comes from that locally running Electron test; it is not a mockup or generated image.

Real Maka Workbar registered tool launcher

Base sync

Rebased the single feature commit without conflict onto apache/main at a6fd57ba8, including #4741, which removes the unrelated layout-tier E2E that failed the first CI attempt. No functional Workbar code changed during the rebase.

Review focus

The registry remains private to the Workbar feature. It does not introduce a plugin API or move Side Conversation/Terminal lifecycle ownership into the surface layer.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex audited the existing Workbar boundaries, implemented the descriptor refactor and regression coverage, rebased the single feature commit onto the updated base, and ran the listed verification. The human contributor remains responsible for review and submission.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Sep 4, 2026
@testikun

testikun commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

CI triage: the only failure was e2e/transcript-scroll.spec.ts:712 (turn-fixture-1turn-prompt-rail-111). The exact test passed on the original branch both once and in a 10-run repeat (11/11 total). #4707 tracks the identical Linux/Xvfb timing failure and records that an unchanged PR head recovered on a full failed-job rerun. This PR does not touch transcript scrolling or paging, so no unrelated workaround was added.

The single Workbar commit is now rebased without conflict onto apache/main at 19ac204ab, including #4741, which removed that layout-tier E2E from the supported CI baseline. Post-rebase Workbar Node tests (14/14), the focused Electron E2E (1/1), Desktop typecheck/build, and base-aware renderer architecture checks (71/71) all pass. The force-with-lease push triggered a fresh CI run.

CI triage and base-sync update posted with OpenAI Codex assistance on behalf of @testikun.

@testikun
testikun force-pushed the codex/issue-4744-workbar-descriptors branch from dd77ac3 to 10d5f8d Compare September 4, 2026 08:40
@testikun

testikun commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

The post-rebase CI reached Desktop E2E after architecture, lint, format, build, typecheck, knip, and affected workspace tests passed. Its sole failure was the unrelated existing e2e/quote-selection.spec.ts drag-selection case (window.getSelection()?.isCollapsed remained true). On this PR head, the exact test passed 10/10 locally with --repeat-each=10 --workers=1; the Workbar-focused Electron test also passes. No quote-selection or transcript code is touched by this PR. Since main has advanced again, I am rebasing onto the new tip to keep the branch current and trigger a fresh complete run.

@testikun
testikun force-pushed the codex/issue-4744-workbar-descriptors branch from 10d5f8d to b67c360 Compare September 4, 2026 08:57
Converge Workbar labels, icons, launcher entries, and panel rendering on one exhaustive typed registry while preserving existing singleton and dynamic resource behavior.

Generated-by: OpenAI Codex
@testikun
testikun force-pushed the codex/issue-4744-workbar-descriptors branch from b67c360 to 2f1a9be Compare September 5, 2026 02:06
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the careful work here, the behavior-equivalence is real: I traced launcher order, shortcuts, icons, lazy panels and the key={tab.id} reuse and none of them move. I am still going to close this one, for two reasons that are about shape, not execution.

The real duplication this PR targets is already fixed in flight, by deletion. The only place main genuinely repeats tab-kind authority is the launcher: order, shortcut and icon are copied from WORKBAR_TOOL_DEFINITIONS, and WorkbarToolDefinition.icon has no consumer at all (and work-board carries the wrong value). #4789 rewrites the launcher as WORKBAR_TOOL_DEFINITIONS.map(...) with the icon field driving the glyph, and it removes the tasks kind, the hand-written tab strip and the DnD layer (−1739 lines). It touches all four files this PR touches. Landing this first would only add a rebase and then leave WORKBAR_SURFACE_DESCRIPTOR_BY_KIND as an intermediate state to delete again.

The rest is a second registry, not a smaller surface. tabLabel (needs ordinal/title/count) and panel rendering (each panel has different props) are not tabular, so moving them into label/render members keeps the same branching in a different container, and WorkbarSurfaceRenderContext becomes a second props channel that forces WorkbarSurfaceProps to be exported. Net: +114 lines, one more registry, nine new names, for a plugin-registration need that this PR itself lists as a non-goal. The remaining descriptor.kind === 'side-chat' check at workbar-surface.tsx:792 shows the table did not absorb the conditionals either.

Two smaller notes for the record, since the tests would need to change regardless: the boundary test asserts source text with exact indentation, which Biome would break without any behavior change, and exhaustiveness is already enforced by the mapped type; and the new Electron case duplicates accessibility-coverage.spec.ts (walks every launcher entry) plus the existing Terminal/Side Chat cases, in the opposite direction from #4761.

I will note on #4331 that the launcher half of "registered tab descriptor model" lands with #4789, and the rest should wait until third-party panels are actually scheduled. Closing.

AI-assisted review: drafted with Maka; I verified the launcher equivalence, the #4789 overlap and the dead icon field myself.

@Astro-Han Astro-Han closed this Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(desktop): register Workbar tab surface descriptors

2 participants