Skip to content

refactor(desktop): centralize session navigation ports - #4449

Merged
Astro-Han merged 3 commits into
apache:mainfrom
orangeCatDeveloper:refactor/session-navigation-port-types
Sep 3, 2026
Merged

refactor(desktop): centralize session navigation ports#4449
Astro-Han merged 3 commits into
apache:mainfrom
orangeCatDeveloper:refactor/session-navigation-port-types

Conversation

@orangeCatDeveloper

@orangeCatDeveloper orangeCatDeveloper commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Refs #4395

Summary

Session Navigation row actions redeclare the remove outcome, toast API, and ref holder already owned by the feature boundary, so those contracts can drift without a type error at their consumers.

The feature now declares its shell ports and supporting contracts once in ports.ts, with ref holders typed as React's RefObject. Row actions consume the canonical remove outcome instead of restating the Host result, while the public and testing entry points continue exporting SessionNavigationPorts under the same name. The toast port stays a hand-written narrow slice rather than a Pick of @maka/ui's ToastApi: structural typing already checks it against the real implementation where the shell wires it in.

This is type-only. The main-process Runtime Host translation remains separate, and ref holders outside Session Navigation are deliberately unchanged.

Verification

Before
controller/session-row-actions.ts:26:type RefBox<T> = { current: T };
controller/session-row-actions.ts:36:type SessionRemoveOutcome = {
controller/session-row-actions.ts:41:type ToastApi = {
controller/use-session-navigation-controller.ts:47:export type SessionNavigationToastApi = {
controller/use-session-navigation-controller.ts:64:type RefBox<T> = { current: T };

After
ports.ts:31:export interface SessionNavigationRemoveOutcome {
ports.ts:40:export type SessionNavigationToastApi = {
ports.ts:112:export interface SessionNavigationPorts {
ports.ts:113:  activeIdRef: RefObject<string | undefined>;

npm --workspace @maka/desktop run build:main passed. The six Session Navigation suites passed 46 tests, and the renderer architecture check passed 60 tests. Renderer typecheck, Biome lint, and formatting also passed.

The complete Desktop typecheck still reaches three unrelated @maka/ui errors already present on main: removed autoScroll, settledText, and trailingAction props. This PR does not change those files.

AI use

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

Tool(s) and scope: OpenAI Codex identified the duplicate feature contracts, implemented the type-only consolidation, and ran the listed checks.

Checklist

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

No new tests: this is a type-only consolidation with no runtime behavior to
exercise. The existing Session Navigation suites cover the consumers and were
run. Full Desktop typecheck was not clean — see the three pre-existing
@maka/ui errors noted under Verification.

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@orangeCatDeveloper
orangeCatDeveloper marked this pull request as ready for review September 1, 2026 08:57
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 1, 2026
@orangeCatDeveloper
orangeCatDeveloper force-pushed the refactor/session-navigation-port-types branch from 678ba00 to e71c1d0 Compare September 1, 2026 09:52
The toast port stays a hand-written narrow slice: structural typing
already checks it against @maka/ui's ToastApi where the shell wires it
in, and deriving it via Pick would leak that API's return types into
every test stub.

Generated-by: OpenAI Codex
@orangeCatDeveloper
orangeCatDeveloper force-pushed the refactor/session-navigation-port-types branch from cc0a5c6 to ea56ecc Compare September 3, 2026 08:37

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked the two things a centralization like this can get wrong, and neither is here.

No old declaration site survives inside the feature: SessionNavigationToastApi and RefBox now resolve only through ports.ts. And no call-site semantics were flattened, the SessionNavigationPorts field list is identical before and after. SessionRemoveOutcome was a genuine duplicate, SessionNavigationRemoveOutcome already existed in ports.ts on main with the same doc comment.

Two details worth recording since they are the ones that could have bitten: RefBox<T> to RefObject<T> is type-identical under the pinned @types/react ^19.2.18 ({ current: T }, mutable, no implicit | null), though it would have been a real narrowing under React 18 types. And the now-readonly outcome fields change nothing, both consumers destructure without assigning back.

Net removal, one fewer file owning contracts, no new abstraction.

Evidence boundary: read only, no build or typecheck run. The PR's claim that three @maka/ui typecheck errors pre-exist on main is unverified by me.

AI-assisted review: drafted with Maka.

@Astro-Han
Astro-Han merged commit 63c1437 into apache:main Sep 3, 2026
1 check passed
@orangeCatDeveloper
orangeCatDeveloper deleted the refactor/session-navigation-port-types branch September 3, 2026 09:47
ggbdpq pushed a commit to ggbdpq/maka that referenced this pull request Sep 4, 2026
Three navigation contracts were declared twice, or declared in a controller file that did not own them. `ToastApi` and `SessionNavigationToastApi` were identical shapes in two files, `SessionRemoveOutcome` duplicated the `SessionNavigationRemoveOutcome` that already lived in `ports.ts`, and `RefBox` was declared separately in both controller files.

Move all of them to the feature's ports module, which is where the shell contract belongs, and use React's own `RefObject` instead of the local `RefBox` alias. Both entry points re-export `SessionNavigationPorts` under the same name, so no importer changes.

No behavior change. The port field list is identical, and the outcome's new `readonly` fields are safe because both consumers destructure without assigning back.

Generated-by: OpenAI Codex

Generated-by: GLM-5.3-Flash (ZCode)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants