Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 61 additions & 8 deletions src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,29 @@
// Suppresses the safe-mode auto-clear watcher while "Roll back to last
// known good" programmatically selects a (necessarily older) version.
const suppressSafeModeAutoClear = ref(false)
/**
* Ceiling for the three non-blocking loaders fired after the app list renders
* (advisories, pins, policies).
*
* Without one, `fetch` waits forever. Measured (issue #160): all three were
* left permanently suspended at their `await fetch`, so `pins` never left `{}`
* and pin badges, advisory badges and auto-update policy state were silently
* absent — with no error, no console output and no failed request, because a
* promise that never settles reaches neither the success path nor the catch.
*
* 8s is far above any healthy response here (every other endpoint on this page
* answers in ~60ms) so a timeout means something is genuinely wrong, and the
* catch then reports it instead of the UI quietly missing a feature.
*
* ⚠️ It was 20s, and that made the abort UNOBSERVABLE: the e2e assertion that
* watches for the pin badge gives up at 15s, so the timeout fired after the
* test had already failed and the catch never ran within the window. A bound
* must fit inside the bound that contains it — the same arithmetic error as a
* retry that outlasts its job cap. Keep this below the 15s expect timeout in
* playwright.config.ts.
*/
const BACKGROUND_FETCH_TIMEOUT_MS = 8_000

const safeModeStorageKey = 'app_versions_safe_mode'
const debugModeStorageKey = 'app_versions_debug_mode'
const dryRunStorageKey = 'app_versions_dry_run_mode'
Expand Down Expand Up @@ -189,12 +212,12 @@
// action is activated; see "Hits route into existing flows".
const sourcesPrefill = ref<PrefillBindPayload | null>(null)

/**

Check warning on line 215 in src/App.vue

View workflow job for this annotation

GitHub Actions / lint-check

Missing JSDoc @PARAM "appId" declaration

Check warning on line 215 in src/App.vue

View workflow job for this annotation

GitHub Actions / quality / Vue Quality (eslint)

Missing JSDoc @PARAM "appId" declaration
* Routes an installed Discover hit into the Apps tab with its version picker
* expanded; see "Hits route into existing flows" ("Installed hit opens the
* picker").
*
* @spec openspec/specs/app-discovery/spec.md

Check warning on line 220 in src/App.vue

View workflow job for this annotation

GitHub Actions / lint-check

Invalid JSDoc tag name "spec"

Check warning on line 220 in src/App.vue

View workflow job for this annotation

GitHub Actions / quality / Vue Quality (eslint)

Invalid JSDoc tag name "spec"
*/
const onDiscoverOpenApp = async (appId: string): Promise<void> => {
currentTab.value = 'apps'
Expand All @@ -202,12 +225,12 @@
await onPickApp(appId)
}

/**

Check warning on line 228 in src/App.vue

View workflow job for this annotation

GitHub Actions / lint-check

Missing JSDoc @PARAM "payload" declaration

Check warning on line 228 in src/App.vue

View workflow job for this annotation

GitHub Actions / quality / Vue Quality (eslint)

Missing JSDoc @PARAM "payload" declaration
* Routes a not-installed Discover hit's installable candidate into the
* Sources bind flow, prefilled; see "Hits route into existing flows"
* ("Installable candidate prefills bind").
*
* @spec openspec/specs/app-discovery/spec.md

Check warning on line 233 in src/App.vue

View workflow job for this annotation

GitHub Actions / lint-check

Invalid JSDoc tag name "spec"

Check warning on line 233 in src/App.vue

View workflow job for this annotation

GitHub Actions / quality / Vue Quality (eslint)

Invalid JSDoc tag name "spec"
*/
const onDiscoverPrefillBind = (payload: PrefillBindPayload): void => {
sourcesPrefill.value = payload
Expand All @@ -218,7 +241,7 @@
* Routes a non-installable Discover hit to the Trusted sources tab; see
* "Hits route into existing flows" ("Non-installable explains why").
*
* @spec openspec/specs/app-discovery/spec.md

Check warning on line 244 in src/App.vue

View workflow job for this annotation

GitHub Actions / lint-check

Invalid JSDoc tag name "spec"

Check warning on line 244 in src/App.vue

View workflow job for this annotation

GitHub Actions / quality / Vue Quality (eslint)

Invalid JSDoc tag name "spec"
*/
const onDiscoverOpenTrusted = (): void => {
currentTab.value = 'trusted'
Expand Down Expand Up @@ -467,7 +490,7 @@
// appears once this resolves. Read-only — it never changes a version.
const loadAdvisories = async (): Promise<void> => {
try {
const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/advisories')), { headers: { ...ocsHeaders, Accept: 'application/json' } })
const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/advisories')), { headers: { ...ocsHeaders, Accept: 'application/json' }, signal: AbortSignal.timeout(BACKGROUND_FETCH_TIMEOUT_MS) })
const payload = await unwrapOcsResponse<{ advisories: Record<string, AdvisoryCorrelation> }>(response)
advisories.value = payload.advisories || {}
} catch {
Expand Down Expand Up @@ -505,15 +528,21 @@
// banner / the pin-override dialog. See "Honest pin presentation".
const loadPins = async (): Promise<void> => {
try {
const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/pins')), { headers: { ...ocsHeaders, Accept: 'application/json' } })
const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/pins')), { headers: { ...ocsHeaders, Accept: 'application/json' }, signal: AbortSignal.timeout(BACKGROUND_FETCH_TIMEOUT_MS) })
const payload = await unwrapOcsResponse<{ pins: PinRecord[] }>(response)
const map: Record<string, PinRecord> = {}
for (const pin of payload.pins || []) {
map[pin.appId] = pin
}
pins.value = map
} catch {
} catch (error) {
// NOT a bare `catch {}`. A silent catch here is why this took seven
// eliminated hypotheses to chase: pins ends up `{}` and the app-card
// badge simply never renders, with nothing anywhere saying why — no
// failed request, no page error, no console output (issue #160).
pins.value = {}
// eslint-disable-next-line no-console
console.error('[app_versions] loadPins failed; pin badges will not render:', error)
}
}

Expand All @@ -535,7 +564,7 @@
// writes go through onPolicyChange()/saveAutoUpdateSettings().
const loadPolicies = async (): Promise<void> => {
try {
const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/policies')), { headers: { ...ocsHeaders, Accept: 'application/json' } })
const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/policies')), { headers: { ...ocsHeaders, Accept: 'application/json' }, signal: AbortSignal.timeout(BACKGROUND_FETCH_TIMEOUT_MS) })
const payload = await unwrapOcsResponse<{ policies?: PolicyRecord[], autoUpdateEnabled?: boolean, autoUpdateWindow?: string }>(response)
const map: Record<string, PolicyRecord> = {}
for (const policy of payload.policies || []) {
Expand Down Expand Up @@ -1389,14 +1418,37 @@
try {
await checkUpdateChannel()
await loadApps()
} catch (error) {
// A `finally` WITHOUT a `catch` re-throws, and the three non-blocking
// loaders below are plain statements in the same function — so anything
// thrown here silently skipped ALL of them.
//
// Measured on CI (issue #160): the browser requested update-channel and
// apps, then NOTHING — no /api/pins, no /api/advisories, no
// /api/policies. `pins` therefore stayed `{}` and the app-card badge's
// `v-if="pinFor(app.id)"` never matched, which is why
// pinning.spec.ts:70 failed on every run since the suite began running.
//
// Catching does not depend on knowing WHAT throws: whatever it is, the
// page must still load its pins, advisories and policies. The message is
// surfaced rather than swallowed so the underlying throw stays visible.
errorMessage.value = error instanceof Error ? error.message : 'Could not initialise the app list.'
} finally {
isLoading.value = false
}
// Kick off advisory correlation, pin state, and auto-update policies after
// the list renders (non-blocking).
void loadAdvisories()
void loadPins()
void loadPolicies()
// the list renders (non-blocking). Each handles its own failures; the extra
// catch guards a SYNCHRONOUS throw before the first await, which would
// otherwise take the following calls down with it.
//
// ⚠️ These three do NOT currently complete — see issue #160. Traced with
// console markers: execution demonstrably reaches this line and all three
// are invoked, yet none reaches its success path or its catch, so each is
// still parked on its `await fetch`. Consequence: pin badges, advisory
// badges and auto-update policy state are silently absent.
void loadAdvisories().catch(() => undefined)
void loadPins().catch(() => undefined)
void loadPolicies().catch(() => undefined)
})

watch([safeModeEnabled, installedVersion, selectedVersion], () => {
Expand Down Expand Up @@ -1618,6 +1670,7 @@
<article
v-for="app in filteredApps"
:key="app.id"
:data-app-id="app.id"
:class="[$style.appCard, { [$style.appCardSelected]: selectedApp === app.id, [$style.appCardCore]: app.isCore }]">
<div :class="$style.appCardBody">
<div :class="$style.appCardHeader">
Expand Down
32 changes: 31 additions & 1 deletion tests/e2e/pinning.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,41 @@ test.describe('version pinning', () => {
})
expect(put.ok(), 'seeding a pin via API should succeed').toBeTruthy()

// The badge renders `v-if="pinFor(app.id)"`, fed by GET /api/pins. So
// "element(s) not found" has three quite different causes — the pin was
// never stored, the list endpoint does not return it, or the component
// did not render it — and the locator alone cannot tell them apart.
// Reading the API first turns one opaque failure into a statement about
// WHICH link of that chain broke.
const pinsList = await page.request.get('/ocs/v2.php/apps/app_versions/api/pins?format=json', {
headers: { 'OCS-APIRequest': 'true' },
})
const listed = (await pinsList.json())?.ocs?.data?.pins ?? []
expect(
listed.map((p: { appId: string }) => p.appId),
`GET /api/pins must list the pin just seeded for "${APP}" — if this passes and the badge below does not appear, the break is in the component, not the API`,
).toContain(APP)

await openSettings(page)
await openTab(page, 'Apps')

// Located by the card's OWN app id, not by its visible text. The card
// renders `{{ app.label }}`, so a text match proves a label contains the
// string — it says nothing about `app.id`, which is the value the
// badge's `pinFor(app.id)` actually keys on. Those are the two sides of
// the comparison under test, so matching on the wrong one would make a
// green assertion meaningless.
const card = page.locator(`article[data-app-id="${APP}"]`)
await expect(
card,
`no app card has data-app-id="${APP}" — the pin is keyed by appId, so if the card's id differs from the API's appId that mismatch IS the bug`,
).toBeVisible()

const badge = page.getByTestId('pin-badge').first()
await expect(badge).toBeVisible()
await expect(
badge,
`the API lists a pin for "${APP}" and its card IS rendered (both asserted above), so the badge's own v-if="pinFor(app.id)" is what did not match — compare the pin's appId against the card's app.id`,
).toBeVisible()
await expect(badge).toContainText('Pinned')
// Attribution is carried in the title so hovering explains the badge.
await expect(badge).toHaveAttribute('title', /admin/)
Expand Down
Loading