experiments in a post-browser web
10
fork

Configure Feed

Select the types of activity you want to include in your feed.

docs(agent-report): Phase 2.5 #3 final — audit + dead-code cleanup landed, tests green

+73 -63
+73 -63
AGENT_REPORT.md
··· 1 - # Phase 2.5 #3 Agent Report — createExtensionWindow migration audit 1 + # Phase 2.5 #3 Agent Report — createExtensionWindow migration audit + dead-code cleanup 2 2 3 - ## Audit (Phase A) 3 + ## Audit findings (Phase A) 4 4 5 - Target: `createExtensionWindow()` in `backend/electron/main.ts` (line 1106). 6 - Still uses v1 `preload.js` (`config.preloadPath`). Function creates a 7 - BrowserWindow per extension and loads `peek://ext/{id}/background.html`. 5 + Target: `createExtensionWindow()` in `backend/electron/main.ts`. Still uses v1 6 + `preload.js`. All 8 call sites classified: 8 7 9 - ### Call sites (6 inside main.ts, 2 inside ipc.ts) 8 + | Site | Caller | Reachability | Bucket | 9 + |------|--------|--------------|--------| 10 + | main.ts `loadEnabledExtensions()` | none | DEAD (exported but unreferenced; replaced by `loadExtensions()` at entry.ts:741) | dead code | 11 + | main.ts `loadExtensions()` — `externalBuiltinIds` loop | startup | EMPTY in practice — all 31 features in `features/` are `manifestVersion: 2` and get loaded by `loadV2Feature()` into `v2FeatureIds`, so the filter `!CONSOLIDATED ∩ !v2FeatureIds` yields nothing | dead in practice | 12 + | main.ts `loadExtensions()` — `enabledExternalExts` loop | startup | EMPTY — `extensions` DB table is only populated by the legacy settings-pane `api.extensions.add()` flow, superseded by features-manager (which writes to the v2 feature registry) | dead in practice | 13 + | main.ts `loadDevExtension()` | CLI `--load-extension` | LIVE (dev-only) | v1 path, live | 14 + | main.ts `reloadExtension()` | features-manager UI via `api.extensions.reload()` | LIVE | v1 path, live | 15 + | ipc.ts `extension-window-load` handler | none | DEAD | dead code | 16 + | ipc.ts `extension-window-reload` handler | none | DEAD | dead code | 10 17 11 - | Site | Caller | Runtime reachability | Bucket | 12 - |------|--------|----------------------|--------| 13 - | main.ts:1213 | `loadEnabledExtensions()` | **DEAD** — exported but never invoked. Replaced by `loadExtensions()` (entry.ts:741). | dead code | 14 - | main.ts:1458 | `loadExtensions()` — `externalBuiltinIds` loop | **EMPTY in practice.** Filter = `registered ∩ !CONSOLIDATED_EXTENSION_IDS ∩ enabled ∩ !v2FeatureIds`. All 35 features in `features/` are v2 (`manifestVersion: 2`) and get loaded by `loadV2Feature()` into `v2FeatureIds`, so this set is empty. | dead in practice | 15 - | main.ts:1467 | `loadExtensions()` — `enabledExternalExts` from `extensions` DB table | Empty under normal test / dev. The `extensions` table is populated by the legacy settings UI `api.extensions.add()` — superseded by features-manager (which installs into the v2 feature registry, not this table). | dead in practice | 16 - | main.ts:1672 | `loadDevExtension()` | Live when user passes `--load-extension` CLI flag. Dev-only. | v1 path, dev-only | 17 - | main.ts:1747 | `reloadExtension()` | **LIVE.** Called by `ipcMain.handle('extension-reload')` (ipc.ts:1374), which is invoked by `api.extensions.reload(id)` on trustedBuiltin tiles (features-manager, test fixtures). | v1 path, live | 18 - | ipc.ts:1287 | `ipcMain.handle('extension-window-load')` | **DEAD** — no renderer invokes this channel (grep). | dead code | 19 - | ipc.ts:1315 | `ipcMain.handle('extension-window-reload')` | **DEAD** — no renderer invokes this channel (grep). | dead code | 18 + Bucket counts: **0** v2-migratable (already migrated via `loadV2Tile`), **0** 19 + chrome extensions (those use `session.loadExtension` in 20 + `chrome-extensions.ts`, not `createExtensionWindow`), **2** live v1 callers 21 + that need a `relaunchTile(tileId)` helper that doesn't exist yet, **4** dead 22 + call sites safe to remove. 20 23 21 - ### Bucket summary 24 + ## Implementation choice + rationale 22 25 23 - - **v2-migratable builtin externals:** 0. All features are already loaded through 24 - `loadV2Tile()` / `launchTile()` today — `createExtensionWindow` is never 25 - invoked for them at runtime. 26 - - **Chrome extensions:** 0 — Chrome extensions use a separate path 27 - (`backend/electron/chrome-extensions.ts` + `session.loadExtension()`), not 28 - `createExtensionWindow`. Out of scope as planned. 29 - - **v1-only live callers:** 2 — `reloadExtension` (features-manager) and 30 - `loadDevExtension` (CLI). Both need a v2 "launch/relaunch a tile by id" 31 - helper that doesn't exist yet (tile-launcher has `launchTile(opts)` but no 32 - "find the manifest, revoke the old token, close the old window, relaunch" 33 - convenience). 34 - - **Dead code:** 4 call sites (`loadEnabledExtensions`, two IPC handlers, 35 - + the two startup branches that are empty in practice). Safe to drop. 26 + The common assumption in the v1-removal plan — "`createExtensionWindow` exists 27 + for external builtins like `example`" — is outdated. `example` is already a v2 28 + tile (eagerly launched via its `type: "background"` entry), and 29 + `createExtensionWindow` is dead on the startup path. 36 30 37 - ### Implementation choice 31 + The two remaining LIVE callers (`reloadExtension`, `loadDevExtension`) each 32 + need a v2 "launch a tile by id, revoking any existing window/token" helper — 33 + tile-launcher currently exposes `launchTile(opts)` that takes a parsed 34 + manifest, not an id + reload-aware-by-default. Building that is a dedicated 35 + Phase 2.5 #3b (est. 1–2h). 38 36 39 - **Exit at audit boundary.** Migrating `reloadExtension` and `loadDevExtension` 40 - requires: 37 + For this pass: **commit the audit**, then **remove the dead code** so Phase 3 38 + destructive cleanup has a smaller surface when the remaining two live callers 39 + are migrated. 41 40 42 - 1. A `relaunchTile(tileId)` helper in `tile-launcher.ts` that re-reads the 43 - manifest from disk, revokes the old token, closes the old window, and 44 - calls `launchTile` again. 45 - 2. A "find the tile's resident/launch entry from manifest" utility (currently 46 - only `loadV2Tile` does this, and it's tangled with eager/lazy branching). 47 - 3. Handling the dev-extension case where the path is registered 48 - transiently via `registerDevExtension` and not in the feature registry. 41 + ## What landed 49 42 50 - That's 1–2 commits of new infrastructure, not a "remove v1 call site" swap. 51 - Given the 60-minute budget and the audit finding that `createExtensionWindow` 52 - is effectively already replaced for the common path (startup loading), the 53 - correct move is to (a) remove the dead-code call sites cleanly in a follow-up 54 - and (b) defer the remaining two live v1 call sites to a dedicated Phase 2.5 #3b. 43 + Three commits on top of `28d07a05`: 55 44 56 - ### Open items for Phase 3 / follow-up 45 + 1. `cb7b2778` — `chore: triage stub for Phase 2.5 #3` 46 + 2. (merged into audit commit) `chore: audit createExtensionWindow callers` 47 + 3. `25c154ea` — `refactor(electron): remove dead createExtensionWindow callers` 57 48 58 - - **Phase 2.5 #3b** — implement `relaunchTile(tileId)` in tile-launcher.ts; 59 - rewrite `reloadExtension()` and `loadDevExtension()` to use it; delete 60 - `createExtensionWindow`, `loadEnabledExtensions`, the dead IPC handlers, 61 - and the dead `externalBuiltinIds` / `enabledExternalExts` branches. 62 - - Phase 3 can proceed for everything not behind `createExtensionWindow` — 63 - features-manager reload and CLI dev extensions are the only surfaces 64 - that still need v1 `preload.js`. 49 + Files changed in the refactor commit: 65 50 66 - ### Files read during audit 51 + - `backend/electron/main.ts`: 52 + - Deleted `loadEnabledExtensions()` function (51 lines, dead). 53 + - Demoted `createExtensionWindow()` from `export` to module-private and 54 + added a doc-comment explaining its remaining live callers + pointing to 55 + Phase 2.5 #3b for the full removal. 56 + - `backend/electron/ipc.ts`: 57 + - Deleted `ipcMain.handle('extension-window-load')` (14 lines). 58 + - Deleted `ipcMain.handle('extension-window-reload')` (16 lines). 59 + - Removed the now-unused `createExtensionWindow` import. 60 + - `backend/electron/index.ts`: dropped `createExtensionWindow` and 61 + `loadEnabledExtensions` from the public surface. 62 + - `app/index.js`: updated stale "calls loadEnabledExtensions()" comment to 63 + point at the real startup path. 67 64 68 - - `backend/electron/main.ts` — createExtensionWindow, loadExtensions, loadEnabledExtensions, reloadExtension, loadDevExtension, discoverBuiltinExtensions 69 - - `backend/electron/ipc.ts` — extension-window-* handlers, extension-reload 70 - - `backend/electron/tile-compat.ts` — loadV2Tile, ensureTileIpcHandlers 71 - - `backend/electron/tile-loader.ts` — loadV2Feature, loadFeaturesFromRegistry 72 - - `backend/electron/feature-startup.ts` — initializeFeatures 73 - - `backend/electron/feature-installer.ts` — syncBuiltinFeatures 74 - - `backend/electron/tile-launcher.ts` — launchTile 75 - - `backend/electron/extensions.ts` — getExternalExtensions (reads `extensions` DB table) 76 - - All `features/*/manifest.json` — confirmed 31 of 31 are `manifestVersion: 2` 65 + Net LOC delta: −110 / +20. Public exports shrink by 2. 77 66 78 67 ## Validation 79 68 80 - N/A at audit-commit. No code changes landed. 69 + - `npx tsc -p backend/tsconfig.json --noEmit` — clean. 70 + - `yarn test:unit` — 565/565 pass. 71 + - `yarn test:grep "v2 trustedBuiltin tile"` — 1/1. 72 + - `yarn test:grep "direct pubsub kagi execution works"` — 1/1. 73 + - `yarn test:grep "open and close settings"` — 1/1. 74 + - `yarn test:grep "Group Mode Context"` — 5/5. 75 + - `yarn test:grep "HUD"` — 11/11. 76 + - `yarn test:grep "example"` — 1 test, skipped (not a regression; pre-existing). 77 + 78 + ## Open items for Phase 3 / follow-up 79 + 80 + - **Phase 2.5 #3b** — add `relaunchTile(tileId)` in `tile-launcher.ts` 81 + (re-read manifest, revoke old token via `revokeTokensForTile`, close window 82 + via `tileWindows` map, call `launchTile` with the resident/launch entry). 83 + Rewrite `reloadExtension()` and `loadDevExtension()` on top of it. Then 84 + delete `createExtensionWindow`, the remaining dead branches in 85 + `loadExtensions()` (`externalBuiltinIds` + `enabledExternalExts` loops), 86 + and the `extensionWindows` map. 87 + - **Chrome extensions** stay on `session.loadExtension()` — documented in 88 + `docs/v1-removal-plan.md` and not part of v1 removal. 89 + - `extension-window-unload` IPC handler is probably also dead but intentionally 90 + left in this pass — scoped audit confirmed `-load` / `-reload` only.