Skip to content

refactor(ui): commit extension runtime authority - #891

Merged
benvinegar merged 4 commits into
mainfrom
refactor/extension-runtime-main
Aug 28, 2026
Merged

refactor(ui): commit extension runtime authority#891
benvinegar merged 4 commits into
mainfrom
refactor/extension-runtime-main

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

  • restack PR refactor(ui): extract extension runtime lifecycle #882's extension runtime lifecycle extraction onto current main
  • publish selection, navigation, command, and provider authority only after React commits
  • retire review- and registry-bound controls on reload, replacement, and unmount
  • preserve frozen command selection, live navigation, and attributed command failure containment
  • keep command/menu/keymap orchestration in App

Test-first regression

Before applying the implementation, the new App-level regression reproduced both leaks on fresh main:

  • a Suspense sibling abandoned App's render but left its uncommitted eventContextProvider installed
  • registry replacement left the predecessor provider installed

The red run was 0/2; both tests pass with this extraction. A separate focused assertion now proves navigation uses successor committed callbacks rather than a captured predecessor.

Validation

  • bun run typecheck
  • bun run deps:check
  • bun run format:check
  • bun run lint
  • full unit suite — 1,810 pass, 8 skip
  • full PTY integration — 132 pass, 1 platform skip
  • TTY smoke — 9 pass
  • real-TTY committed-diff smoke
  • two independent reviews with no remaining findings

This includes an empty maintenance changeset.

This PR description was generated by Pi using gpt-5.6-sol

@vercel

vercel Bot commented Aug 28, 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 Aug 28, 2026 12:51pm

Request Review

@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR extracts extension command execution, event-context publication, and committed runtime authority from App into dedicated hooks. It adds regression coverage for abandoned renders, registry replacement, remount retirement, committed navigation bindings, selection snapshots, and command failure containment.

  • Publishes extension runtime bindings and event providers only after React commits.
  • Separates render-time pane projections from committed command and navigation authority.
  • Revokes review- and registry-bound controls on reload, replacement, and unmount.
  • Adds focused hook and App-level lifecycle tests.

Confidence Score: 4/5

The runtime refactor appears safe to merge after the non-blocking filename convention violations are corrected.

The lifecycle and authority changes are guarded by commit-time effects and capability leases with focused regression coverage; the only accepted issue is that several new TypeScript filenames do not follow the repository's enforced naming convention.

Files Needing Attention: src/ui/hooks/useExtensionRuntimeBridge.ts and the other newly added camelCase/PascalCase TypeScript files

Important Files Changed

Filename Overview
src/ui/App.tsx Replaces inline extension authority and command composition with committed runtime, provider, and runner hooks.
src/ui/hooks/useExtensionRuntimeBridge.ts Introduces committed refs, capability leases, render/commit projections, and post-commit command/navigation bindings; its filename violates the repository convention.
src/ui/hooks/useExtensionEventContextProvider.ts Installs the event-context provider in a layout effect and uses identity-checked cleanup during replacement and unmount; its filename violates the repository convention.
src/ui/hooks/useExtensionCommandRunner.ts Centralizes extension command-context construction and synchronous/asynchronous failure containment; its filename violates the repository convention.
src/ui/App.extension-runtime.test.tsx Adds App-level regressions for abandoned-render provider publication and provider retirement; its filename violates the repository convention.
src/ui/hooks/useExtensionRuntimeBridge.test.tsx Exercises runtime liveness, registry and review replacement, frozen selection, and committed navigation behavior; its filename violates the repository convention.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Render[App render] --> Bridge[Extension runtime bridge]
  Bridge -->|layout commit| Facts[Commit registry, review, and selection facts]
  Facts --> Bindings[Commit commands and navigation]
  Bindings --> Provider[Install event-context provider]
  Provider --> Events[AppHost publishes lifecycle events]
  Bridge --> Commands[Extension command runner]
  Commands --> Snapshot[Frozen selection and review controls]
  Commands --> Live[Live committed navigation]
  Reload[Reload or registry replacement] --> Revoke[Revoke predecessor leases]
  Revoke --> Bridge
Loading
Prompt To Fix All With AI
### Issue 1
src/ui/hooks/useExtensionRuntimeBridge.ts:1
**Use dash-case TypeScript filenames**

This file and the other newly added hook and test files use camelCase or PascalCase filename segments, violating the repository's enforced dash-case convention and causing filename-case lint failures. Rename them to dash-case and update their imports.

---

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

Reviews (1): Last reviewed commit: "test(ui): distinguish committed navigati..." | Re-trigger Greptile

@@ -0,0 +1,264 @@
/**

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 Use dash-case TypeScript filenames

This file and the other newly added hook and test files use camelCase or PascalCase filename segments, violating the repository's enforced dash-case convention and causing filename-case lint failures. Rename them to dash-case and update their imports.

Context Used: guidelines.mdc Cursor rule (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/hooks/useExtensionRuntimeBridge.ts
Line: 1

Comment:
**Use dash-case TypeScript filenames**

This file and the other newly added hook and test files use camelCase or PascalCase filename segments, violating the repository's enforced dash-case convention and causing filename-case lint failures. Rename them to dash-case and update their imports.

**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.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 merged commit 3127c19 into main Aug 28, 2026
13 checks passed
@benvinegar
benvinegar deleted the refactor/extension-runtime-main branch August 28, 2026 12:59
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