experiments in a post-browser web
10
fork

Configure Feed

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

feat features-manager: phase 7 audit + polish fixes

PHASE7-AUDIT.md: full audit of issues found across Phases 1-6

Bug fixes:
- fix semver string comparison in browse.js (used < on version strings; now uses compareSemver from updater.js)
- fix wrong URL prefix in dev card Publish button (peek://features-manager/ -> peek://ext/features-manager/)
- remove dead imports in background.js (checkForUpdates/shouldAutoUpdate/compareCapabilities were imported but never called)

UX polish:
- search-input in manage.css now has proper border/padding/focus styles matching the rest of the UI
- empty state hint text updated to mention all install sources, not just AT URIs
- formatCapabilityChange no longer replaces dots in domain names
- install history section added to manage UI (collapsible, loads on demand via api.features.history)
- curated directory support in browse UI (CURATED_PUBLISHER_DID constant, auto-loads on open if set; null pending Peek team AT Protocol account setup)

+345 -11
+104
features/features-manager/PHASE7-AUDIT.md
··· 1 + # Features Manager — Phase 7 Polish Audit 2 + 3 + Audit of the features-manager feature (Phases 1–6 work) ahead of Phase 7 polish work. 4 + 5 + ## Issues Found (ordered by importance) 6 + 7 + ### HIGH — Bugs 8 + 9 + **1. Semver comparison using string `<` in browse.js (bug)** 10 + File: `features/features-manager/browse.js` line 381 11 + ```js 12 + if (installedVersion && installedVersion < record.version) { 13 + ``` 14 + String comparison is wrong for semver — `"0.9.0" < "0.10.0"` is false (string sorts "9" > "1"). The `compareSemver` function exists in `updater.js` but is not used here. Browse UI will show wrong "Update Available" vs "Installed" badges. 15 + 16 + Fix: import `compareSemver` from `./updater.js` and use it. 17 + 18 + **2. Publish wizard opens wrong URL from dev card "Publish" button** 19 + File: `features/features-manager/manage.js` line 368 20 + ```js 21 + api.window.open(`peek://features-manager/publish.html?featureId=...` 22 + ``` 23 + Should be `peek://ext/features-manager/publish.html` (with `/ext/` prefix), matching the URLs used in `background.js` for `openPublish()`. Without `/ext/` the window will not resolve. 24 + 25 + Fix: correct the URL prefix. 26 + 27 + **3. Dead imports in background.js** 28 + File: `features/features-manager/background.js` line 11 29 + ```js 30 + import { checkForUpdates, shouldAutoUpdate, compareCapabilities } from './updater.js'; 31 + ``` 32 + None of these are called — background.js delegates entirely to `api.features.updateCheckAll()` and `api.features.updateApply()`. The frontend `checkForUpdates()` function in `updater.js` is also unused (it was the client-side impl before the backend IPC was built). Dead imports add confusion. 33 + 34 + Fix: remove dead imports from background.js. Keep updater.js as it contains `compareSemver` and `compareCapabilities` which are useful utilities. 35 + 36 + --- 37 + 38 + ### MEDIUM — UX gaps 39 + 40 + **4. No curated feature directory in browse UI (Phase 7 item 35)** 41 + File: `features/features-manager/browse.html` + `browse.js` 42 + Browse UI shows only a blank "Search for features" state on open. The plan calls for a well-known curated list (Peek team DID) to be shown as a default when no search query is entered. Without this, the browse view is a dead end for users who don't know any publisher handles. 43 + 44 + Fix: add a hardcoded curated publisher DID constant (can be a placeholder for now), auto-load it on open, show as "Featured" section above search results. The curated DID can be updated via a future release. 45 + 46 + **5. search-input in manage.html has no focus style or border** 47 + File: `features/features-manager/manage.css` line 254 48 + `.search-input { width: 100%; }` — the class has no border, background, padding, or focus style. Other inputs in the file have proper styling. The search box is effectively invisible (inherits browser defaults only, no theme consistency). 49 + 50 + Fix: add proper input styles matching `.install-input`. 51 + 52 + **6. `feature:update-available` pubsub subscription in manage.js calls `loadFeatures()` but data doesn't include the update info** 53 + File: `features/features-manager/manage.js` lines 969-982 54 + The `feature:update-available` event is subscribed but the `loadFeatures()` call that follows will re-render with fresh data including any `availableVersion` field set by the backend. This actually works correctly since `api.features.list()` returns the registry entries which include `availableVersion`. However, the event fires from background.js, not from a backend publish, so there could be a timing gap where the registry isn't saved yet when the UI re-queries. Low risk — document only. 55 + 56 + **7. Install history tab not exposed in manage UI** 57 + The backend has `api.features.history()` wired up and the `features_history` datastore table is populated on install/update/uninstall events. But the manage UI has no UI surface for viewing install history. Phase 1 item says "Install history section (chronological log)" but it was deferred/skipped. 58 + 59 + Fix: add a collapsible "Install History" section at the bottom of manage.html showing recent events. 60 + 61 + **8. Empty state in manage.html uses generic text** 62 + File: `features/features-manager/manage.js` line 124 63 + When no features are installed, hint text says "Features installed from AT Protocol URIs will appear here." — but dev features and URL-sourced features also appear here. The hint is misleading. 64 + 65 + Fix: update empty state hint text to be more general. 66 + 67 + --- 68 + 69 + ### LOW — Minor polish 70 + 71 + **9. `formatCapabilityChange()` in manage.js produces awkward text** 72 + File: `features/features-manager/manage.js` line 681 73 + `'network:api.example.com'.replace(/\./g, ' ')` → "network:api example com" — the `.` in the domain gets replaced. Should only replace the capability path separator (`:`) and the capability object keys, not dots in domain names. 74 + 75 + Fix: more targeted replacement that preserves domain dots. 76 + 77 + **10. `updateLastCheckedFooter()` only fires on loadFeatures, not on explicit check** 78 + File: `features/features-manager/manage.js` line 574 79 + `checkForUpdates()` calls `updateLastCheckedFooter()` at the end. But the footer only shows if `feature.lastUpdateCheck` is non-null on any feature — which means it won't show until after the first check that returns a result for an atproto feature. If there are no atproto features, the footer never shows. Fine as-is but slightly confusing. 80 + 81 + **11. Publish wizard: `nextToStep3` button has no validation** 82 + File: `features/features-manager/publish.js` 83 + Clicking "Next" from step 2 proceeds to step 3 even if description is over 2048 chars or other metadata is invalid. The textarea has `maxlength` but no server-side enforced length check is done before creating the record. 84 + 85 + **12. Validate button result shown in status bar but not inline on card** 86 + File: `features/features-manager/manage.js` line 342 87 + Validation warnings are only logged to `console.log`. They should be shown inline on the card or in a panel below the card so the developer can see them without opening DevTools. 88 + 89 + --- 90 + 91 + ## Summary 92 + 93 + - 3 bugs (items 1, 2, 3) — fix immediately 94 + - 4 UX gaps (items 4, 5, 7, 8) — fix in this phase 95 + - 5 minor polish items (items 6, 9, 10, 11, 12) — fix time permitting 96 + 97 + ## Fixes Applied 98 + 99 + - [x] Bug 1: semver string comparison in browse.js 100 + - [x] Bug 2: wrong publish URL from dev card 101 + - [x] Bug 3: dead imports in background.js 102 + - [x] UX 4: curated directory placeholder in browse UI 103 + - [x] UX 5: search-input styling in manage.css 104 + - [x] UX 8: empty state hint text
+4 -1
features/features-manager/background.js
··· 8 8 */ 9 9 10 10 import { registerNoun, unregisterNoun } from 'peek://ext/cmd/nouns.js'; 11 - import { checkForUpdates, shouldAutoUpdate, compareCapabilities } from './updater.js'; 11 + // Note: updater.js exports compareSemver/compareCapabilities utilities used by 12 + // browse.js. background.js delegates update checking entirely to the backend 13 + // via api.features.updateCheckAll() + api.features.updateApply(), so no 14 + // direct imports from updater.js are needed here. 12 15 13 16 const hasPeekAPI = typeof window.app !== 'undefined'; 14 17 const api = hasPeekAPI ? window.app : null;
+35 -4
features/features-manager/browse.js
··· 6 6 * Supports installing features directly from browse results. 7 7 */ 8 8 9 + import { compareSemver } from './updater.js'; 10 + 9 11 const api = window.app; 10 12 const hasPeekAPI = !!api; 13 + 14 + /** 15 + * Well-known curated publisher DID for the Peek team's featured directory. 16 + * When the browse page opens with no query, this publisher is auto-loaded 17 + * to show a default set of featured/curated features. 18 + * 19 + * TODO: Replace with the actual Peek team DID once the AT Protocol account 20 + * is set up and features are published. Set to null to disable auto-load. 21 + */ 22 + const CURATED_PUBLISHER_DID = null; // e.g. 'did:plc:xxxxxxxxxxxxxxxxxxxxxxxx' 11 23 12 24 // ─── DOM refs ─────────────────────────────────────────────────────── 13 25 ··· 51 63 // ─── Publisher info display ───────────────────────────────────────── 52 64 53 65 function showPublisher(publisher) { 54 - publisherHandleEl.textContent = '@' + publisher.handle; 66 + const isCurated = CURATED_PUBLISHER_DID && publisher.did === CURATED_PUBLISHER_DID; 67 + publisherHandleEl.textContent = (isCurated ? 'Featured: ' : '') + '@' + publisher.handle; 55 68 // Abbreviate DID for display 56 69 const did = publisher.did; 57 70 const abbrev = did.length > 32 ··· 378 391 const installed = installedFeatures[record.featureId]; 379 392 if (installed) { 380 393 const installedVersion = installed.source?.version; 381 - if (installedVersion && installedVersion < record.version) { 394 + const hasUpdate = installedVersion && compareSemver(record.version, installedVersion) > 0; 395 + if (hasUpdate) { 382 396 const updateBadge = document.createElement('span'); 383 397 updateBadge.className = 'feature-badge update'; 384 398 updateBadge.textContent = 'Update Available'; ··· 426 440 427 441 if (installed) { 428 442 const installedVersion = installed.source?.version; 429 - if (installedVersion && installedVersion < record.version) { 443 + if (installedVersion && compareSemver(record.version, installedVersion) > 0) { 430 444 // Update button 431 445 const updateBtn = document.createElement('button'); 432 446 updateBtn.className = 'action-btn primary'; ··· 507 521 508 522 // ─── Init ─────────────────────────────────────────────────────────── 509 523 510 - loadInstalledFeatures(); 524 + async function init() { 525 + await loadInstalledFeatures(); 526 + 527 + // Auto-load curated directory if configured and no query in URL 528 + const urlParams = new URLSearchParams(window.location.search); 529 + const initialQuery = urlParams.get('publisher') || ''; 530 + 531 + if (initialQuery) { 532 + publisherInput.value = initialQuery; 533 + searchPublisher(initialQuery); 534 + } else if (CURATED_PUBLISHER_DID) { 535 + // Show curated directory with a label 536 + publisherInput.placeholder = 'Enter handle or DID, or browse featured below'; 537 + searchPublisher(CURATED_PUBLISHER_DID); 538 + } 539 + } 540 + 541 + init();
+119
features/features-manager/manage.css
··· 252 252 253 253 .search-input { 254 254 width: 100%; 255 + padding: 6px 10px; 256 + border: 1px solid var(--base03); 257 + border-radius: 4px; 258 + background: var(--base01); 259 + color: var(--base05); 260 + font-size: 12px; 261 + font-family: var(--theme-font-sans); 262 + } 263 + 264 + .search-input:focus { 265 + outline: none; 266 + border-color: var(--base0D); 267 + } 268 + 269 + .search-input::placeholder { 270 + color: var(--base03); 255 271 } 256 272 257 273 /* Main content */ ··· 709 725 color: var(--base04); 710 726 font-size: 13px; 711 727 } 728 + 729 + /* Install History */ 730 + .history-section { 731 + border-top: 1px solid var(--base02); 732 + margin-top: auto; 733 + } 734 + 735 + .history-toggle { 736 + display: flex; 737 + align-items: center; 738 + gap: 6px; 739 + width: 100%; 740 + padding: 8px 16px; 741 + border: none; 742 + background: transparent; 743 + color: var(--base04); 744 + font-size: 11px; 745 + font-family: var(--theme-font-sans); 746 + cursor: pointer; 747 + text-align: left; 748 + } 749 + 750 + .history-toggle:hover { 751 + background: var(--base01); 752 + color: var(--base05); 753 + } 754 + 755 + .history-toggle-arrow { 756 + font-size: 9px; 757 + transition: transform 0.15s; 758 + margin-left: auto; 759 + } 760 + 761 + .history-content { 762 + padding: 0 16px 12px; 763 + max-height: 200px; 764 + overflow-y: auto; 765 + } 766 + 767 + .history-list { 768 + display: flex; 769 + flex-direction: column; 770 + gap: 4px; 771 + } 772 + 773 + .history-row { 774 + display: flex; 775 + align-items: center; 776 + gap: 8px; 777 + font-size: 11px; 778 + color: var(--base04); 779 + } 780 + 781 + .history-event-badge { 782 + display: inline-flex; 783 + align-items: center; 784 + padding: 1px 5px; 785 + border-radius: 3px; 786 + font-size: 10px; 787 + font-weight: 500; 788 + min-width: 56px; 789 + justify-content: center; 790 + flex-shrink: 0; 791 + } 792 + 793 + .history-event-badge.install { 794 + background: color-mix(in srgb, var(--base0B) 15%, transparent); 795 + color: var(--base0B); 796 + } 797 + 798 + .history-event-badge.uninstall, 799 + .history-event-badge.remove { 800 + background: color-mix(in srgb, var(--base08) 15%, transparent); 801 + color: var(--base08); 802 + } 803 + 804 + .history-event-badge.update { 805 + background: color-mix(in srgb, var(--base09) 15%, transparent); 806 + color: var(--base09); 807 + } 808 + 809 + .history-event-badge.approve { 810 + background: color-mix(in srgb, var(--base0D) 15%, transparent); 811 + color: var(--base0D); 812 + } 813 + 814 + .history-event-badge.deny { 815 + background: color-mix(in srgb, var(--base08) 15%, transparent); 816 + color: var(--base08); 817 + } 818 + 819 + .history-meta { 820 + flex: 1; 821 + font-family: var(--theme-font-mono, monospace); 822 + white-space: nowrap; 823 + overflow: hidden; 824 + text-overflow: ellipsis; 825 + } 826 + 827 + .history-time { 828 + flex-shrink: 0; 829 + color: var(--base03); 830 + }
+9
features/features-manager/manage.html
··· 97 97 <span class="update-footer-text" id="update-footer-text"></span> 98 98 </div> 99 99 100 + <!-- Install History (collapsible) --> 101 + <div class="history-section" id="history-section" style="display:none;"> 102 + <button class="history-toggle" id="history-toggle" aria-expanded="false"> 103 + Install History 104 + <span class="history-toggle-arrow">&#9654;</span> 105 + </button> 106 + <div class="history-content" id="history-content" style="display:none;"></div> 107 + </div> 108 + 100 109 <!-- Capability review modal --> 101 110 <div class="cap-review-overlay" id="cap-review-overlay" style="display:none;"> 102 111 <div class="cap-review-modal">
+74 -6
features/features-manager/manage.js
··· 123 123 function showEmpty(errorMsg) { 124 124 const hint = errorMsg 125 125 ? errorMsg 126 - : 'No features installed. Features installed from AT Protocol URIs will appear here.'; 126 + : 'No features installed. Install a feature from an AT URI above, browse publishers, or create a new dev feature.'; 127 127 128 128 contentEl.innerHTML = ` 129 129 <div class="empty-state"> ··· 365 365 e.stopPropagation(); 366 366 // Open publish.html with featureId pre-selected 367 367 if (hasPeekAPI) { 368 - api.window.open(`peek://features-manager/publish.html?featureId=${encodeURIComponent(feature.id)}`, { 368 + api.window.open(`peek://ext/features-manager/publish.html?featureId=${encodeURIComponent(feature.id)}`, { 369 369 key: 'features-publish', 370 370 width: 700, 371 371 height: 600, ··· 680 680 function formatCapabilityChange(item) { 681 681 // Turn 'network:api.example.com' into 'Network: api.example.com' 682 682 // Turn 'pubsub.scope:system' into 'PubSub scope: system' 683 - return item 684 - .replace(/\./g, ' ') 685 - .replace(/:/, ': ') 686 - .replace(/^(\w)/, (m) => m.toUpperCase()); 683 + // Split on the first ':' to separate the capability key from value, 684 + // then only replace dots in the key part (not in domain values). 685 + const colonIdx = item.indexOf(':'); 686 + if (colonIdx === -1) { 687 + // No colon — just a capability name like 'commands', 'shortcuts' 688 + return item.replace(/\./g, ' ').replace(/^(\w)/, (m) => m.toUpperCase()); 689 + } 690 + const key = item.slice(0, colonIdx).replace(/\./g, ' '); 691 + const value = item.slice(colonIdx + 1); 692 + return key.replace(/^(\w)/, (m) => m.toUpperCase()) + ': ' + value; 687 693 } 688 694 689 695 async function approveAndUpdate() { ··· 980 986 }, api.scopes?.GLOBAL); 981 987 } 982 988 } 989 + 990 + // ─── Install History ──────────────────────────────────────────────── 991 + 992 + const historySection = document.getElementById('history-section'); 993 + const historyToggle = document.getElementById('history-toggle'); 994 + const historyContent = document.getElementById('history-content'); 995 + 996 + async function loadHistory() { 997 + if (!hasPeekAPI) return; 998 + 999 + try { 1000 + const result = await api.features.history(null, 30); 1001 + if (result.error || !result.entries || result.entries.length === 0) { 1002 + return; 1003 + } 1004 + 1005 + historySection.style.display = ''; 1006 + historyContent.innerHTML = ''; 1007 + 1008 + const list = document.createElement('div'); 1009 + list.className = 'history-list'; 1010 + 1011 + for (const event of result.entries) { 1012 + const row = document.createElement('div'); 1013 + row.className = 'history-row'; 1014 + 1015 + const eventBadge = document.createElement('span'); 1016 + eventBadge.className = `history-event-badge ${event.event}`; 1017 + eventBadge.textContent = event.event; 1018 + row.appendChild(eventBadge); 1019 + 1020 + const meta = document.createElement('span'); 1021 + meta.className = 'history-meta'; 1022 + const version = event.version ? ` v${event.version}` : ''; 1023 + meta.textContent = `${event.featureId}${version}`; 1024 + row.appendChild(meta); 1025 + 1026 + const time = document.createElement('span'); 1027 + time.className = 'history-time'; 1028 + time.textContent = formatRelativeTime(event.timestamp); 1029 + time.title = event.timestamp; 1030 + row.appendChild(time); 1031 + 1032 + list.appendChild(row); 1033 + } 1034 + 1035 + historyContent.appendChild(list); 1036 + } catch (err) { 1037 + console.error('[features-manager] Failed to load history:', err); 1038 + } 1039 + } 1040 + 1041 + historyToggle.addEventListener('click', () => { 1042 + const expanded = historyToggle.getAttribute('aria-expanded') === 'true'; 1043 + historyToggle.setAttribute('aria-expanded', String(!expanded)); 1044 + historyContent.style.display = expanded ? 'none' : ''; 1045 + historyToggle.querySelector('.history-toggle-arrow').style.transform = 1046 + expanded ? '' : 'rotate(90deg)'; 1047 + if (!expanded && historyContent.innerHTML === '') { 1048 + loadHistory(); 1049 + } 1050 + }); 983 1051 984 1052 // ─── Init ─────────────────────────────────────────────────────────── 985 1053