Skip to content

refactor(ui): extract extension runtime lifecycle - #882

Merged
benvinegar merged 2 commits into
refactor/extension-pane-controllerfrom
refactor/extension-runtime-bridge
Aug 27, 2026
Merged

refactor(ui): extract extension runtime lifecycle#882
benvinegar merged 2 commits into
refactor/extension-pane-controllerfrom
refactor/extension-runtime-bridge

Conversation

@benvinegar

@benvinegar benvinegar commented Aug 27, 2026

Copy link
Copy Markdown
Member

Summary

  • move extension runtime authority and command context construction out of App
  • install event-context providers only after commit and retire stale capabilities during layout cleanup
  • preserve frozen command selection, live navigation, and soft- and hard-reload authority

Testing

  • focused runtime bridge, provider lifecycle, and command runner tests
  • relevant App and AppHost extension lifecycle tests
  • PTY extension integration tests
  • bun run typecheck
  • bun run deps:check
  • bun run lint
  • targeted formatting and diff checks

Stack

This PR description was generated by Pi using GPT-5.6 Sol

@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hunk-web Ignored Ignored Preview Aug 27, 2026 12:14pm

Request Review

@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR extracts extension command execution, event-context installation, and runtime capability lifecycle management from App into focused hooks.

  • Introduces a runtime bridge for committed selection projections, command/navigation bindings, and capability retirement.
  • Moves extension command context construction and error containment into a dedicated hook.
  • Installs event contexts during layout commit with identity-checked cleanup.
  • Adds focused tests for StrictMode, reload, remount, registry replacement, selection snapshots, and command failures.

Confidence Score: 4/5

The PR appears safe to merge, with only a non-blocking test-fixture typing issue to address.

The extracted runtime lifecycle preserves committed-state ordering and stale-capability retirement, while the only accepted concern is that new tests bypass structural fixture checking through empty-object type assertions.

Files Needing Attention: src/ui/hooks/useExtensionCommandRunner.test.tsx and src/ui/hooks/useExtensionEventContextProvider.test.tsx

Important Files Changed

Filename Overview
src/ui/hooks/useExtensionRuntimeBridge.ts Centralizes committed extension state, selection projections, capability leases, and command/navigation bindings with lifecycle-focused tests.
src/ui/hooks/useExtensionCommandRunner.ts Extracts command-context construction and consistently contains synchronous, asynchronous, and context-construction failures.
src/ui/hooks/useExtensionEventContextProvider.ts Installs event contexts during layout commit and prevents stale cleanup from removing a successor provider.
src/ui/App.tsx Replaces inline extension lifecycle machinery with the three extracted hooks while retaining host composition responsibilities.
src/ui/hooks/useExtensionCommandRunner.test.tsx Thoroughly covers command context and failure containment, but its partial fixtures bypass the repository’s required typed fixture helper.
src/ui/hooks/useExtensionEventContextProvider.test.tsx Covers commit timing, StrictMode, replacement, and cleanup while repeating the partial-fixture assertion issue.
src/ui/hooks/useExtensionRuntimeBridge.test.tsx Exercises retirement and liveness across StrictMode, hard remounts, content reloads, registry replacement, and selection changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  App[App render] --> Bridge[Extension runtime bridge]
  App --> Runner[Command runner]
  App --> Provider[Event context provider]
  App --> Bindings[Command and navigation bindings]
  Bindings -->|layout commit| Bridge
  Bridge --> Lease[Registry and review leases]
  Bridge --> Projection[Committed and render projections]
  Runner --> Context[Extension command context]
  Provider --> Events[Lifecycle and custom events]
  Lease -->|reload or unmount| Retired[Retired controls become inert]
Loading
Prompt To Fix All With AI
### Issue 1
src/ui/hooks/useExtensionCommandRunner.test.tsx:20
**Partial mocks bypass typing**

The new hook tests construct partial extension-capability fixtures with `{} as ...` assertions instead of the required `@total-typescript/shoehorn` helpers. This bypasses structural checking and allows capability contracts to change without useful compile-time failures; the same pattern appears in `useExtensionEventContextProvider.test.tsx`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "refactor(ui): isolate extension runtime ..." | Re-trigger Greptile

import { createEmptyExtensionLoadResult, type RegisteredCommand } from "../../extensions/types";
import { useExtensionCommandRunner } from "./useExtensionCommandRunner";

const commandControls = {} as ExtensionCommandControls;

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.

P2 Partial mocks bypass typing

The new hook tests construct partial extension-capability fixtures with {} as ... assertions instead of the required @total-typescript/shoehorn helpers. This bypasses structural checking and allows capability contracts to change without useful compile-time failures; the same pattern appears in useExtensionEventContextProvider.test.tsx.

Context Used: testing.mdc Cursor rule (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/hooks/useExtensionCommandRunner.test.tsx
Line: 20

Comment:
**Partial mocks bypass typing**

The new hook tests construct partial extension-capability fixtures with `{} as ...` assertions instead of the required `@total-typescript/shoehorn` helpers. This bypasses structural checking and allows capability contracts to change without useful compile-time failures; the same pattern appears in `useExtensionEventContextProvider.test.tsx`.

**Context Used:** testing.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/testing.mdc))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@benvinegar
benvinegar force-pushed the refactor/extension-pane-controller branch from acd7b42 to b0b60d1 Compare August 27, 2026 12:13
@benvinegar
benvinegar force-pushed the refactor/extension-runtime-bridge branch from 55e431c to 8bde834 Compare August 27, 2026 12:14
@benvinegar
benvinegar merged commit 559cd22 into refactor/extension-pane-controller Aug 27, 2026
12 checks passed
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.

1 participant