mirror of
https://github.com/dsh-market/dsh-market.git
synced 2026-09-28 05:03:07 +08:00
* fix(routes): a failed enable must not flip the durable patch layer (#575) The toggle route wrote the patch rows unconditionally, so a failed enable (deterministic import crash) flipped the durable layer to enabled and turned a transient in-session error into a boot crash loop. Gate the enable-direction patch write on ok; disables keep their unconditional write (a failed unmount still means the user asked for OFF). * fix(routes): a failed enable must not flip the durable patch layer (#575) The toggle route wrote the patch rows unconditionally, so a failed enable (deterministic import crash) flipped the durable layer to enabled and turned a transient in-session error into a boot crash loop. Gate the enable-direction patch write on ok; disables keep their unconditional write (a failed unmount still means the user asked for OFF). * fix(routes): a failed enable must not flip the durable patch layer (#575) The toggle route wrote the patch rows unconditionally, so a failed enable (deterministic import crash) flipped the durable layer to enabled and turned a transient in-session error into a boot crash loop. Gate the enable-direction patch write on ok; disables keep their unconditional write (a failed unmount still means the user asked for OFF). * test: pin the toggle patch-layer gate (#575) Bundle-plugin flow: a failed enable leaves the user patch untouched (still disabled) instead of flipping the durable layer into a boot crash loop. * fix: a failed enable leaves every view of the state as it was (#575) Completes @JINITAIMEI121's fix in #584, whose diagnosis and asymmetry are kept verbatim: a failed ENABLE must not flip the durable layer, a failed DISABLE must — the user asked for OFF, and a failed unmount leaves the plugin live only for this session. Their patch-layer gate closed the hole in cordis.patch.yml. Two of the three views the report described were still flipped: setPluginEnabled recorded the choice BEFORE attempting the mount and persisted state.json whatever happened, so after a failed enable the market's own store and its in-memory set both said "enabled" while the patch layer said "disabled". That left two problems. The persisted views disagreed, so which one won at the next boot depended on load order — harder to diagnose than the original bug. And a CLIENT-ONLY plugin has no bundle rows at all, so `patchRows` is empty, the patch gate never runs, and state.json is its only durable state: for that plugin kind the crash loop was untouched. The reporter measured 24 restarts. The in-memory set is now restored before persisting, so the reply, the store and the patch layer tell one story. Tests: the bundle-plugin case also asserts the market's own answer, and a new client-only case covers the path the patch gate cannot reach. Both failed before this change with `expected [] to include …`. Co-authored-by: JINITAIMEI121 <JINITAIMEI121@users.noreply.github.com> --------- Co-authored-by: eeeeeeaaaa12 <1303576427@qq.com> Co-authored-by: JINITAIMEI121 <JINITAIMEI121@users.noreply.github.com>
This commit is contained in:
fkysly
co-authored by
JINITAIMEI121
eeeeeeaaaa12
JINITAIMEI121
eeeeeeaaaa12
parent
1ecdd8eaa0
commit
310ac4a58c
+41
-6
@@ -487,14 +487,31 @@ export function mountMarketRoutes(
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply one enable/disable request: persist the choice in state.json, then
|
||||
* drive the live composition. Covers every mount form — hot mounts and
|
||||
* client-only shims go through hotUnmount/hotMount, bundle-layer entries
|
||||
* through setEntryDisabled. Enabling a THEME goes through the caller's
|
||||
* activateTheme instead so the Themes tab's exclusivity stays intact.
|
||||
* Apply one enable/disable request: drive the live composition, then
|
||||
* persist the choice in state.json. Covers every mount form — hot mounts
|
||||
* and client-only shims go through hotUnmount/hotMount, bundle-layer
|
||||
* entries through setEntryDisabled. Enabling a THEME goes through the
|
||||
* caller's activateTheme instead so the Themes tab's exclusivity stays
|
||||
* intact.
|
||||
*
|
||||
* A FAILED ENABLE LEAVES EVERYTHING AS IT WAS (#575). The choice used to be
|
||||
* recorded before the mount was attempted and persisted whatever happened,
|
||||
* so enabling a plugin that crashes on import — deterministically, every
|
||||
* time — wrote "enabled" into state.json anyway. The next boot tried the
|
||||
* import again and died again; the reporter measured 24 restarts before
|
||||
* restoring the disable by hand. The toggle route's patch-layer gate
|
||||
* (@JINITAIMEI121 in #584) closed the same hole in cordis.patch.yml; this
|
||||
* closes it in the market's own store, which is the ONLY durable state a
|
||||
* client-only plugin has — that plugin kind has no bundle rows, so the
|
||||
* patch gate never runs for it.
|
||||
*
|
||||
* A failed DISABLE still persists, and that asymmetry is deliberate: the
|
||||
* user asked for OFF, and a failed unmount leaves the plugin live only for
|
||||
* this session. There the durable disable is the contract, not an error.
|
||||
*/
|
||||
async function setPluginEnabled(name: string, enabled: boolean): Promise<{ ok: boolean; reason?: string }> {
|
||||
const dir = activeProfileDir
|
||||
const wasDisabled = disabled.has(name)
|
||||
if (enabled) disabled.delete(name)
|
||||
else disabled.add(name)
|
||||
let ok: boolean
|
||||
@@ -522,6 +539,13 @@ export function mountMarketRoutes(
|
||||
ok = true
|
||||
}
|
||||
}
|
||||
if (!ok && enabled) {
|
||||
// Put the in-memory view back before persisting: it is the same object
|
||||
// the route reports as `disabled`, so restoring it keeps the reply, the
|
||||
// store and the patch layer telling one story.
|
||||
if (wasDisabled) disabled.add(name)
|
||||
logEvent('warn', 'toggle', `${name}: enable failed; leaving it disabled rather than persisting a state that crashes at boot (#575)`)
|
||||
}
|
||||
writeMarketState(dir, { disabled, groups, groupOrder })
|
||||
return { ok, reason }
|
||||
}
|
||||
@@ -2109,7 +2133,18 @@ export function mountMarketRoutes(
|
||||
}
|
||||
}
|
||||
let patchWrite: { ok: boolean; reason: string | null } | null = null
|
||||
if (patchRows.length > 0) {
|
||||
// #575: a failed ENABLE must not flip the durable patch layer.
|
||||
// The hot-mount failure may be deterministic (a plugin that
|
||||
// crashes on import), and persisting "enabled" turns a transient
|
||||
// in-session error into a boot crash loop — the loader re-applies
|
||||
// the flipped rows on every start. The frontend already shows the
|
||||
// plugin as still disabled, and the next explicit enable retries
|
||||
// cleanly. Disables keep their unconditional write: a failed
|
||||
// unmount leaves the plugin live in-session, and the user asked
|
||||
// for it OFF — the durable disable is then the contract, not an
|
||||
// error.
|
||||
const patchGate = ok || !enabled
|
||||
if (patchRows.length > 0 && patchGate) {
|
||||
for (const rowId of patchRows) {
|
||||
const result = enabled ? await enableRow(userPatchPath, rowId) : await disableRow(userPatchPath, rowId)
|
||||
if (!result.ok && patchWrite === null) patchWrite = result
|
||||
|
||||
+67
-1
@@ -18,7 +18,7 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { dirname, join } from 'node:path'
|
||||
|
||||
// ---------------------------------------------------------------- FakeDsh
|
||||
// Mutable per-test state driving the fake executor and fake npm API.
|
||||
@@ -473,6 +473,7 @@ const REGISTRY = {
|
||||
{ name: 'dsh-usage-stats', owner: 'a2', url: 'https://github.com/a2/dsh-usage-stats', category: 'tool', npm: null, description: {}, install: '', added: '' },
|
||||
{ name: 'dsh-blue-whale', owner: 'o', url: 'https://github.com/o/blue-whale', category: 'tool', npm: null, description: {}, install: '', added: '' },
|
||||
{ name: 'dsh-patchy', owner: 'o', url: 'https://github.com/o/dsh-patchy', category: 'tool', npm: null, description: {}, install: '', added: '' },
|
||||
{ name: 'dsh-crashy', owner: 'o', url: 'https://github.com/o/dsh-crashy', category: 'tool', npm: null, description: {}, install: '', added: '' },
|
||||
// Carries a prebuilt Release archive (#250): its install target is a
|
||||
// URL, not an npm name and not a github: shortcut.
|
||||
{ name: 'dsh-prebuilt', owner: 'o', url: 'https://github.com/o/dsh-prebuilt', category: 'tool', npm: null, tarball: 'https://github.com/o/dsh-prebuilt/releases/download/v1.0.0/dsh-prebuilt.tgz', description: {}, install: '', added: '' },
|
||||
@@ -3815,6 +3816,71 @@ describe('generic enable/disable toggle (#60)', () => {
|
||||
expect(on.json.reason).toMatch(/cannot hot-mount|restart/)
|
||||
})
|
||||
|
||||
it('leaves the patch layer untouched when an enable fails (#575)', async () => {
|
||||
// A bundle plugin with real patch rows (the dsh-plugin-codegraph shape
|
||||
// from the report: enabling it crashes deterministically on import).
|
||||
fake.repos['github:o/dsh-crashy'] = {
|
||||
name: 'dsh-crashy',
|
||||
manifest: { dsh: { bundle: { patch: './cordis.patch.yml' } }, main: 'lib/index.js' },
|
||||
artifacts: ['lib/index.js', 'cordis.patch.yml'],
|
||||
}
|
||||
const installed = await bed.dispatch('POST', '/dsh-market/install', { url: 'https://github.com/o/dsh-crashy' })
|
||||
hot.mounts = []
|
||||
const bundlePatch = join(profileDir('web'), 'node_modules', 'dsh-crashy', 'cordis.patch.yml')
|
||||
mkdirSync(dirname(bundlePatch), { recursive: true })
|
||||
writeFileSync(bundlePatch, "- insert:\n - id: dsh-crashy\n name: 'dsh-crashy'\n - id: dsh-crashy-tool\n name: 'dsh-crashy-tool'\n")
|
||||
// Disable once: the user patch now durably holds the disabled rows.
|
||||
const off = await bed.dispatch('POST', '/dsh-market/toggle', { name: 'dsh-crashy', enabled: false })
|
||||
expect(off.status).toBe(200)
|
||||
const patchPath = join(profileDir('web'), 'cordis.patch.yml')
|
||||
const before = readFileSync(patchPath, 'utf8')
|
||||
expect(before).toContain('- id: dsh-crashy\n disabled: true')
|
||||
|
||||
// The enable fails in-session (the deterministic import crash of #575).
|
||||
// The enable fails: with every row disabled at boot the loader holds no
|
||||
// entry for the plugin (the real #575 shape), so the themes path finds
|
||||
// nothing and the hotMount fallback is what fails.
|
||||
hot.failNext = true
|
||||
const on = await bed.dispatch('POST', '/dsh-market/toggle', { name: 'dsh-crashy', enabled: true })
|
||||
expect(on.status).toBe(502)
|
||||
|
||||
// The durable patch layer must be untouched: persisting the flipped
|
||||
// rows would turn the transient in-session failure into a boot crash
|
||||
// loop.
|
||||
const after = readFileSync(patchPath, 'utf8')
|
||||
expect(after).toBe(before)
|
||||
expect(after).toContain('disabled: true')
|
||||
|
||||
// …and so must the market's OWN durable store. The patch layer and
|
||||
// state.json are two persisted views of the same answer; leaving them
|
||||
// disagreeing is worse than the original bug, because which one wins at
|
||||
// the next boot depends on load order.
|
||||
// …and so must the market's OWN durable answer. `disabled` in the reply
|
||||
// is the same array handed to writeMarketState, so asserting it here is
|
||||
// asserting what the next boot reads. Leaving the two persisted views
|
||||
// disagreeing is worse than the original bug: which one wins at the next
|
||||
// boot depends on load order.
|
||||
expect(on.json.disabled).toContain('dsh-crashy')
|
||||
})
|
||||
|
||||
it('a failed enable leaves a CLIENT-ONLY plugin disabled too (#575)', async () => {
|
||||
// The path the patch gate cannot cover: a client-only package has no
|
||||
// bundle rows, so `patchRows` is empty and the gate never runs. Its only
|
||||
// durable state is state.json — which is exactly where the first version
|
||||
// of this fix still wrote "enabled" after a failed mount, leaving the
|
||||
// same crash loop for this kind of plugin.
|
||||
await installNpm('dsh-loop', { client: './client.js' })
|
||||
const off = await bed.dispatch('POST', '/dsh-market/toggle', { name: 'dsh-loop', enabled: false })
|
||||
expect(off.status).toBe(200)
|
||||
expect(off.json.disabled).toContain('dsh-loop')
|
||||
|
||||
hot.failNext = true
|
||||
const on = await bed.dispatch('POST', '/dsh-market/toggle', { name: 'dsh-loop', enabled: true })
|
||||
expect(on.status).toBe(502)
|
||||
|
||||
expect(on.json.disabled).toContain('dsh-loop')
|
||||
})
|
||||
|
||||
it('toggles a client-only shim (dsh.client without dsh.bundle) through the hot path', async () => {
|
||||
await installNpm('dsh-loop', { client: './client.js' })
|
||||
expect(hot.mounts).toEqual(['dsh-loop'])
|
||||
|
||||
Reference in New Issue
Block a user