diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index fd4101d9..361a4bf4 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -7,6 +7,10 @@ this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +- Restore the profile's default agent preset when `dsh-ops` is disabled or uninstalled: runtime preset default writes now exit the active HMR transaction scope and wait for outer HMR transactions and `/package.json.lock` to settle instead of failing with `HMR transactions cannot be nested` or timing out on the profile lock, and `dsh-ops uninstall` cleans up any dangling `selectedDefault: ops-ptc` entry in `cordis.patch.yml` so new sessions no longer fail with `Unknown agent preset: ops-ptc` (#881). +- 在禁用或卸载 `dsh-ops` 时恢复 profile 的默认 agent 预设:运行时默认预设写入现在会退出当前 HMR 事务上下文并等待外层 HMR 事务及 `/package.json.lock` 结束,避免因 `HMR transactions cannot be nested` 或配置锁超时而残留 `selectedDefault: ops-ptc`;同时 `dsh-ops uninstall` 会在卸载后清理 `cordis.patch.yml` 中悬空的 `selectedDefault: ops-ptc`,防止新建会话报 `Unknown agent preset: ops-ptc`(#881)。 +- The ops-ptc shape reminder no longer calls a program that only waits on `tools.wait_agent` (dot or bracket call), or one with two or more dispatch sites such as a search whose result picks the next read, a "single operation". The wait is read only from a real dispatch site, not from comments or strings; plan and bypass checks, latch priority and re-arming are unchanged (#893). +- ops-ptc 形状提醒不再把仅等待 `tools.wait_agent`(点调用或方括号调用)的程序,以及含两个以上派发站点的程序(如由搜索结果决定后续读取)判为 single operation。等待只从真实派发站点识别,不认注释或字符串;plan 与 bypass 判断、latch 优先级与重新启用不变(#893)。 - Native tool construction and the publication refusal queue no longer attach listeners or lifetime effects to the composition root. The evidence observer and result admission subscribe on the caller's context with global dispatch, and the refusal queue falls back to the caller lifetime when the root refuses an effect. dsh-tui 0.20 refuses `root.events.on` and `root.effect` from a plugin activation, and the native-tools row is one, so every `dsh-tui --resume` aborted with "cannot resume session" before the agent mounted. - 原生工具构造与发布拒绝队列不再向组合根挂监听或生命周期 effect:证据观察者与 result admission 改为在调用方上下文订阅并声明 global 分发,拒绝队列在 root 拒绝 effect 时回落到调用方生命周期。dsh-tui 0.20 会拒绝插件激活期内的 `root.events.on` 与 `root.effect`,而原生工具行正是插件激活,导致每次 `dsh-tui --resume` 都在 agent 挂载前报 "cannot resume session"。 diff --git a/src/presets/ops-ptc/register.ts b/src/presets/ops-ptc/register.ts index 4fbb8029..6aa88998 100644 --- a/src/presets/ops-ptc/register.ts +++ b/src/presets/ops-ptc/register.ts @@ -82,6 +82,20 @@ export interface SettingsService { update(namespace: string, patch: Record): Promise } +/** The slice of the host HMR coordinator needed to avoid nested transactions. */ +export interface HmrService { + operations?: Promise + readonly executing?: { + getStore?(): unknown + exit?(callback: () => T): T + } +} + +/** The slice of the host profile context needed to observe its file lock. */ +export interface ProfileContext { + readonly dir?: string +} + /** One parsed loader patch list, as the host's `loadOverlayPatches` returns it. */ export type FragmentLoader = (binName: string, file: string) => readonly unknown[] @@ -119,6 +133,21 @@ export interface ResolvedAppBoot { */ const registered = new WeakSet() +/** + * Operator defaults captured before `ops-ptc` forced its own selection, per registry. + * + * A remount or a second boot that starts while `selectedDefault` is already `ops-ptc` + * would otherwise read `ops-ptc` as the saved selection and lose the operator's earlier + * choice when the row is later disabled or removed. + */ +const capturedDefaults = new WeakMap() + +/** Maximum time a deferred default write waits for an outer profile lock to clear. */ +const HOST_IDLE_WAIT_MS = 10_000 + +/** Polling interval while waiting for `/package.json.lock` to clear. */ +const HOST_LOCK_POLL_MS = 10 + /** The live registration of one injected context. */ interface Slot { release?: () => Promise @@ -127,6 +156,12 @@ interface Slot { previous?: string /** The settings service, kept so teardown can still write while disposing. */ settings?: SettingsService + /** The HMR coordinator, kept so teardown can queue outside a transaction. */ + hmr?: HmrService + /** The profile directory, kept so teardown can wait out the profile lock. */ + profileDir?: string + /** The in-flight force write, if one started during activation. */ + forcing?: Promise } /** One human-readable error message. */ @@ -148,6 +183,65 @@ function settingsOf(ctx: Context): SettingsService | undefined { return service(ctx, 'settings') } +/** The host HMR coordinator, when the profile runs with hot reload. */ +function hmrOf(ctx: Context): HmrService | undefined { + return service(ctx, 'hmr') +} + +/** The directory of the active profile, when the host publishes one. */ +function profileDirOf(ctx: Context): string | undefined { + const dir = service(ctx, 'profile')?.dir + return typeof dir === 'string' && dir !== '' ? dir : undefined +} + +/** + * Run a callback outside the active HMR transaction's AsyncLocalStorage scope. + * + * `settings.update` delegates to `configEditor.edit`, which calls `hmr.runExclusive`. + * When `dsh-ops` is mounted, reconfigured, disabled, or removed inside an outer + * `hmr.runExclusive`, the `executing` store stays `true` across synchronous calls and + * `await` continuations, causing `hmr.runExclusive` to reject with + * `HMR transactions cannot be nested`. Exiting the store lets the settings write + * queue after `hmr.operations` instead of failing (#881). + */ +function runOutsideHmr(hmr: HmrService | undefined, callback: () => T): T { + const executing = hmr?.executing + if (executing?.getStore?.() === true && typeof executing.exit === 'function') { + return executing.exit(callback) + } + return callback() +} + +/** + * Wait until the outer HMR transaction and any outer profile `package.json.lock` finish. + * + * `PluginManager.change` (used by `removeBundle` and `setBundleEnabled`) holds + * `/package.json.lock` across `hmr.runExclusive` calls, while + * `ConfigEditor.edit` acquires `hmr.runExclusive` first and `package.json.lock` + * second. Waiting for both to settle prevents the settings write from entering + * `hmr.runExclusive` while `PluginManager.change` still holds the file lock. + */ +async function waitForHostIdle( + hmr: HmrService | undefined, + profileDir: string | undefined, + inTransaction: boolean, +): Promise { + const lockFile = profileDir !== undefined ? path.join(profileDir, 'package.json.lock') : undefined + const lockHeld = () => lockFile !== undefined && existsSync(lockFile) + if (!inTransaction && !lockHeld()) return + const deadline = Date.now() + HOST_IDLE_WAIT_MS + await new Promise(resolve => setImmediate(resolve)) + while (Date.now() < deadline) { + const operations = hmr?.operations + if (operations !== undefined) { + try { await operations } catch { /* the host owns its own transaction errors */ } + if (hmr?.operations !== undefined && hmr.operations !== operations) continue + } + if (!lockHeld()) return + await new Promise(resolve => setTimeout(resolve, HOST_LOCK_POLL_MS)) + } +} + /** The preset new sessions currently default to. */ function currentDefault(registry: PresetRegistry): string | undefined { try { @@ -215,7 +309,16 @@ async function writeSelectedDefault(ctx: Context, slot: Slot, options: Registrat */ async function forceDefault(ctx: Context, registry: PresetRegistry, slot: Slot, options: RegistrationOptions): Promise { if (currentDefault(registry) === PRESET_ID) return - await writeSelectedDefault(ctx, slot, options, PRESET_ID) + const hmr = slot.hmr ?? hmrOf(ctx) + const profileDir = slot.profileDir ?? profileDirOf(ctx) + const inTransaction = hmr?.executing?.getStore?.() === true + const task = runOutsideHmr(hmr, async () => { + await waitForHostIdle(hmr, profileDir, inTransaction) + if (slot.disposed || currentDefault(registry) === PRESET_ID) return + await writeSelectedDefault(ctx, slot, options, PRESET_ID) + }) + slot.forcing = task + await task } /** @@ -224,10 +327,19 @@ async function forceDefault(ctx: Context, registry: PresetRegistry, slot: Slot, * may have forced is touched; an operator-chosen default is never rewritten. */ async function restoreDefault(ctx: Context, registry: PresetRegistry, slot: Slot, options: RegistrationOptions): Promise { - if (currentDefault(registry) !== PRESET_ID) return - const target = restoreTarget(registry, slot.previous) - if (target === undefined || target === PRESET_ID) return - await writeSelectedDefault(ctx, slot, options, target) + const hmr = slot.hmr ?? hmrOf(ctx) + const profileDir = slot.profileDir ?? profileDirOf(ctx) + const inTransaction = hmr?.executing?.getStore?.() === true + await runOutsideHmr(hmr, async () => { + if (slot.forcing !== undefined) { + try { await slot.forcing } catch { /* forceDefault already warned */ } + } + await waitForHostIdle(hmr, profileDir, inTransaction) + if (currentDefault(registry) !== PRESET_ID) return + const target = restoreTarget(registry, slot.previous) + if (target === undefined || target === PRESET_ID) return + await writeSelectedDefault(ctx, slot, options, target) + }) } /** @@ -453,7 +565,12 @@ export function registerOpsPtcPreset(root: Context, options: RegistrationOptions const activation: object = ctx.fiber ?? ctx if (registered.has(activation)) return registered.add(activation) - const slot: Slot = { disposed: false } + const slot: Slot = { + disposed: false, + settings: settingsOf(ctx), + hmr: hmrOf(ctx), + profileDir: profileDirOf(ctx), + } ctx.effect(() => () => { slot.disposed = true releaseRegistration(ctx, registry, slot, options) @@ -477,8 +594,16 @@ async function activate(ctx: Context, registry: PresetRegistry, slot: Slot, opti // before reading, or the new activation would capture a value about to change. await settlePendingRestore(registry) // Capture the operator's default before anything can force a new one; teardown - // gives this exact selection back. - slot.previous = savedDefault(registry) + // gives this exact selection back. When the profile booted with `ops-ptc` + // already saved on disk, keep any earlier operator selection seen on this + // registry or fall back to the registry's official default on teardown. + const saved = savedDefault(registry) + if (saved !== PRESET_ID) { + capturedDefaults.set(registry, saved) + slot.previous = saved + } else { + slot.previous = capturedDefaults.get(registry) + } const definition = await readOpsPtcDefinition(options, ctx) if (definition === undefined || slot.disposed) return let release: (() => Promise) | undefined diff --git a/src/profile-default-repair.ts b/src/profile-default-repair.ts new file mode 100644 index 00000000..7ccd5bbd --- /dev/null +++ b/src/profile-default-repair.ts @@ -0,0 +1,120 @@ +/** + * Clear a default agent preset that still names `ops-ptc` after the plugin left the profile. + * + * `global.forceDefaultOpsPtc` persists `selectedDefault: ops-ptc` on the + * `agent-preset-registry` row of the profile patch. The row teardown gives it back, + * but a removal that stops the profile first, runs without HMR, or happens while the + * application is closed never reaches that teardown. The host resolves the default as + * `selectedDefault ?? default` without checking that the preset exists, so the leftover + * makes every new session fail with `Unknown agent preset: ops-ptc` (#881). + * + * Only that one line is removed: the registry then falls back to its own `default`, + * exactly as if the operator had never chosen. Any other shape is left untouched and + * reported, never guessed at. + */ +import fs from 'node:fs' +import path from 'node:path' + +/** The profile row whose volatile `selectedDefault` names the default preset. */ +const REGISTRY_ID = 'agent-preset-registry' +/** The preset this package registers at runtime and the leftover it may leave. */ +const PRESET_ID = 'ops-ptc' + +/** One top-level sequence item: `- id: agent-preset-registry`, possibly quoted. */ +const REGISTRY_ITEM = new RegExp(`^-\\s+id:\\s*(['"]?)${REGISTRY_ID}\\1\\s*(?:#.*)?$`, 'u') +/** A nested `id:` line of a top-level item whose first key is something else. */ +const REGISTRY_ID_LINE = new RegExp(`^\\s+id:\\s*(['"]?)${REGISTRY_ID}\\1\\s*(?:#.*)?$`, 'u') +const SELECTED_LEFTOVER = new RegExp(`^(\\s+)selectedDefault:\\s*(['"]?)${PRESET_ID}\\2\\s*(?:#.*)?$`, 'u') +const CONFIG_LINE = /^(\s+)config:\s*(?:#.*)?$/u + +/** What one repair found. */ +export interface DefaultRepair { + /** The profile patch inspected. */ + readonly file: string + /** A `selectedDefault: ops-ptc` line was found on the registry row. */ + readonly dangling: boolean + /** The line was removed (false for a dry run or when nothing was found). */ + readonly changed: boolean +} + +/** Leading spaces of one line. */ +function indentOf(line: string): number { + return /^ */u.exec(line)![0].length +} + +/** + * Locate the leftover in one profile patch text. + * + * Mirrors the host config editor: when the selection was the only `config` key the + * empty `config:` goes too, and a row left with only `id`/`name` is dropped. + * @param text - the `cordis.patch.yml` contents. + * @returns the 0-based line indexes to remove, ascending; empty when there is none. + */ +export function findDanglingDefault(text: string): number[] { + const lines = text.split(/\r?\n/u) + const topLevel = (line: string) => /^-(?:\s|$)/u.test(line) + let start = 0 + while (start < lines.length) { + // Each top-level item starts at column 0 with `- `; the next one ends it. + let end = start + 1 + while (end < lines.length && !topLevel(lines[end]!)) end += 1 + if (topLevel(lines[start]!)) { + const item = lines.slice(start, end) + const isRegistry = REGISTRY_ITEM.test(item[0]!) || item.slice(1).some(line => REGISTRY_ID_LINE.test(line)) + if (isRegistry) { + let config = -1 + for (let index = 1; index < item.length; index += 1) { + const line = item[index]! + if (CONFIG_LINE.test(line)) { config = index; continue } + const selected = SELECTED_LEFTOVER.exec(line) + // Only a key nested directly under `config:` is the registry selection. + if (selected === null || config < 0 || selected[1]!.length <= indentOf(item[config]!)) continue + const children = item.slice(config + 1).filter((child, offset) => config + 1 + offset !== index + && child.trim() !== '' && !child.trim().startsWith('#')) + .filter(child => indentOf(child) > indentOf(item[config]!)) + if (children.length > 0) return [start + index] + const rest = item.filter((other, offset) => offset !== 0 && offset !== config && offset !== index + && other.trim() !== '' && !other.trim().startsWith('#')) + const onlyIdentity = rest.every(other => /^\s+(?:id|name):/u.test(other)) && /^-\s+(?:id|name):/u.test(item[0]!) + if (onlyIdentity) return Array.from({ length: end - start }, (_, offset) => start + offset) + return [start + config, start + index] + } + } + } + start = end + } + return [] +} + +/** + * Remove a dangling `selectedDefault: ops-ptc` from one profile's patch. + * @param profileDir - the profile directory holding `cordis.patch.yml`. + * @param options - `dryRun` reports without writing. + * @returns what was found and whether the file changed; a missing patch is clean. + */ +export function repairDanglingDefault(profileDir: string, { dryRun = false }: { dryRun?: boolean } = {}): DefaultRepair { + const file = path.join(profileDir, 'cordis.patch.yml') + let text: string + try { text = fs.readFileSync(file, 'utf8') } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return { file, dangling: false, changed: false } + throw error + } + const remove = findDanglingDefault(text) + if (remove.length === 0) return { file, dangling: false, changed: false } + if (dryRun) return { file, dangling: true, changed: false } + const newline = text.includes('\r\n') ? '\r\n' : '\n' + const dropped = new Set(remove) + const lines = text.split(/\r?\n/u).filter((_, index) => !dropped.has(index)) + // The host config editor requires a top-level YAML sequence; when the removed + // entry was the only item, leave an empty sequence behind any header comments. + if (!lines.some(line => /^-(?:\s|$)|^\s*\[/u.test(line))) { + if (lines.length > 0 && lines.at(-1) === '') lines.splice(lines.length - 1, 0, '[]') + else lines.push('[]', '') + } + const mode = fs.statSync(file).mode & 0o777 + // Write beside the target and rename, so a crash never leaves a half-written profile. + const temporary = `${file}.dsh-ops-${process.pid}.tmp` + fs.writeFileSync(temporary, lines.join(newline), { mode }) + fs.renameSync(temporary, file) + return { file, dangling: true, changed: true } +} diff --git a/src/profile-install.ts b/src/profile-install.ts index 29bed1d0..139b7f7b 100644 --- a/src/profile-install.ts +++ b/src/profile-install.ts @@ -6,6 +6,7 @@ import { createRequire } from 'node:module' import { spawnSync } from 'node:child_process' import { PACKAGE_ROOT, dshHome } from './paths.js' import { PLATFORMS } from './execution/runtime.js' +import { repairDanglingDefault } from './profile-default-repair.js' /** * Architectures this fork will run its profile commands on. @@ -173,6 +174,7 @@ export function profileCommand(operation: ProfileOperation, { profile, dshCli, d } else result = spawnSync(command, [...prefix, ...args], { stdio: 'inherit', windowsHide: true }) if (result.error) throw result.error if (result.status !== 0) return result.status ?? 1 + if (operation === 'uninstall') repairDanglingDefault(directory) console.log(operation === 'install' ? 'Installed through the official profile manager. Reload/restart the target application if needed. On Windows the pinned Bash payload ships as an optional dependency; run dsh-ops bash-runtime install to repair a missing payload. No postinstall downloads.' : 'Removed through the official profile manager, including its runtime dependency references. Package-manager shared caches are not deleted.') diff --git a/src/ptc/batching-guidance.ts b/src/ptc/batching-guidance.ts index ed237e6d..c839228e 100644 --- a/src/ptc/batching-guidance.ts +++ b/src/ptc/batching-guidance.ts @@ -82,6 +82,8 @@ const DISPATCH = /\btools\s*(?:\.\s*([A-Za-z_$][\w$]*)|\[\s*['"]([^'"]+)['"]\s*\ const BATCHED = /Promise\.all|Promise\.allSettled|\.map\s*\(|for\s*\(|while\s*\(|forEach\s*\(|\[\s*\.\.\./u const BRANCHING = /\bif\s*\(|\btry\s*\{|\bswitch\s*\(|\?\s*[^:]{1,80}:/u const WAITING = /\bwait\s*:\s*true|job_output\s*\(/u +/** Tools whose call is itself a wait on other work, recognised only at a real dispatch site. */ +const WAITING_TOOL = /^wait_agent$/u const BYPASS = /node:fs|node:child_process|\bfs\./u const SHELL_TOOL = /^(?:pwsh|bash|shell|ops_bash|pwsh_persistent|bash_persistent)$/u @@ -272,7 +274,9 @@ export function analyzeProgram(code: unknown): ProgramShape { dispatchSites: sites.length, hasBatchingStructure: BATCHED.test(code), hasBranching: BRANCHING.test(code), - isWaiting: WAITING.test(code), + // A wait_agent call is a dependency barrier; it counts only as a dispatch site, so + // the name inside a comment or a message string never exempts a program. + isWaiting: WAITING.test(code) || sites.some(match => WAITING_TOOL.test(match[1] ?? match[2] ?? '')), // Comments and message text are not file work; module specifiers stay readable // so a real import of node:fs still bypasses. bypassesFileTools: BYPASS.test(readable.bare) || shellForFiles, @@ -306,8 +310,11 @@ export function violationsOf(shape: ProgramShape): ProgramViolations { plan: !shape.hasFirstLinePlan, bypass: shape.bypassesFileTools, // Waiting on a job and branching are legitimate non-batched shapes, and a long - // program is not judged from its text alone. - shape: !shape.hasBatchingStructure && !shape.hasBranching && !shape.isWaiting && shape.chars < THIN_CODE_CHARS, + // program is not judged from its text alone. The notice claims a single operation, + // so it needs exactly one dispatch site: two sequential sites may be dependent, and + // the text alone cannot prove they could have run in parallel. + shape: shape.dispatchSites === 1 && !shape.hasBatchingStructure && !shape.hasBranching && !shape.isWaiting + && shape.chars < THIN_CODE_CHARS, } } diff --git a/test/batching-trigger.test.mjs b/test/batching-trigger.test.mjs index ce60e224..3e2fdc2d 100644 --- a/test/batching-trigger.test.mjs +++ b/test/batching-trigger.test.mjs @@ -125,6 +125,49 @@ await test('each of the three categories is decided independently', () => { assert.equal(violationsOf(long).shape, false) }) +await test('waiting on an agent is a dependency barrier, read only from a real dispatch site (#893)', () => { + // Dot and bracket calls to wait_agent are both waits, so no shape notice. + for (const code of [ + '// plan: wait for the verification agent\nreturn await tools.wait_agent({ timeout_ms: 30000 })', + '// plan: wait for the verification agent\nreturn await tools["wait_agent"]({ timeout_ms: 30000 })', + ]) { + const shape = analyzeProgram(code) + assert.equal(shape.dispatchSites, 1, code) + assert.equal(shape.isWaiting, true, code) + assert.deepEqual(violationsOf(shape), { plan: false, bypass: false, shape: false }, code) + } + // The name inside a comment or a message string is not a call and exempts nothing. + for (const code of [ + 'const r = await tools.ops_read_files({ files: [{ path: "a" }] })\n// tools.wait_agent({})\nreturn r', + 'const r = await tools.ops_read_files({ files: [{ path: "a" }] })\nreturn { r, note: "tools.wait_agent( later" }', + ]) { + const shape = analyzeProgram(code) + assert.equal(shape.isWaiting, false, code) + assert.equal(violationsOf(shape).shape, true, code) + } + // A wait does not exempt the plan or bypass checks. + assert.equal(violationsOf(analyzeProgram('return await tools.wait_agent({ timeout_ms: 1 })')).plan, true) + const shellWait = analyzeProgram('await tools.ops_bash({ description: "list", command: "ls -la" })\nreturn await tools.wait_agent({})') + assert.equal(violationsOf(shellWait).bypass, true) +}) + +await test('only a single dispatch site earns the single-operation shape notice (#893)', () => { + // A search whose result picks the next read cannot be batched ahead of time. + const dependent = analyzeProgram('// plan: search; read the matching file\nconst hits = await tools.grep({ pattern: "needle" })\nreturn await tools.read({ file_path: hits.matches[0].path })') + assert.equal(dependent.dispatchSites, 2) + assert.equal(dependent.isWaiting, false) + assert.deepEqual(violationsOf(dependent), { plan: false, bypass: false, shape: false }) + // Bracket sites count the same way. + assert.equal(violationsOf(analyzeProgram('const a = await tools["ops_stat"]({ path: "a" })\nreturn await tools["ops_stat"]({ path: a.path })')).shape, false) + // Two sites do not exempt the plan or bypass checks. + const twoNoPlan = analyzeProgram('const a = await tools.ops_bash({ description: "list", command: "ls -la" })\nreturn await tools.ops_stat({ path: "a" })') + assert.deepEqual(violationsOf(twoNoPlan), { plan: true, bypass: true, shape: false }) + // A commented-out second call does not lift a real single site out of the notice. + assert.equal(violationsOf(analyzeProgram('// plan: read\nconst r = await tools.read({ file_path: "a" })\n// await tools.read({ file_path: "b" })\nreturn r')).shape, true) + // The single read keeps its original notice. + assert.equal(violationsOf(analyzeProgram('// plan: read\nreturn await tools.read({ file_path: "a" })')).shape, true) +}) + // ---- the per-turn latch (option 4a: re-armed by compliance, no per-turn cap) ---- diff --git a/test/preset-registration.test.mjs b/test/preset-registration.test.mjs index a58f5be5..16eb196c 100644 --- a/test/preset-registration.test.mjs +++ b/test/preset-registration.test.mjs @@ -6,6 +6,7 @@ * the fragment must yield the full host preset, disposal must release it, one * root must never register twice, and a broken fragment must only warn. */ +import { AsyncLocalStorage } from 'node:async_hooks' import fs from 'node:fs' import { createRequire } from 'node:module' import path from 'node:path' @@ -424,6 +425,90 @@ await test('a live Loader remount keeps the forced default and a disable restore } finally { await shutdown(host.ctx) } }) +await test('live HMR transactions and profile file locks do not block or reject default restoration (#881)', async () => { + await withTempDir('preset-registration-hmr', async profileDir => { + const lockFile = path.join(profileDir, 'package.json.lock') + const hmr = { + operations: Promise.resolve(), + executing: new AsyncLocalStorage(), + runExclusive(operation) { + if (this.executing.getStore() === true) { + return Promise.reject(new Error('HMR transactions cannot be nested')) + } + const run = () => this.executing.run(true, operation) + const task = this.operations.then(run, run) + this.operations = task.then(() => undefined, () => undefined) + return task + }, + } + const events = [] + const makeSettings = registry => ({ + async update(namespace, patch) { + return hmr.runExclusive(async () => { + if (fs.existsSync(lockFile)) { + throw new Error(`Timed out waiting for lock: ${lockFile}`) + } + events.push(`settings:${patch.selectedDefault}`) + registry.state.updates.push({ namespace, patch: { ...patch } }) + registry.state.selected = patch.selectedDefault + }) + }, + }) + + // 1. First boot inside hmr.runExclusive: forceDefault exits the HMR store and persists ops-ptc. + const firstRegistry = stubRegistry() + const firstWarnings = [] + const firstCtx = new Context() + firstCtx.logger.level = 0 + firstCtx.provide('profile', { dir: profileDir }) + firstCtx.provide('hmr', hmr) + firstCtx.provide('agentPresets', firstRegistry) + firstCtx.provide('settings', makeSettings(firstRegistry)) + await hmr.runExclusive(async () => { + registerOpsPtcPreset(firstCtx, { warn: message => firstWarnings.push(message) }) + }) + await waitFor(() => firstRegistry.state.selected === 'ops-ptc', { what: 'the forced default after an HMR boot transaction' }) + assert.deepEqual(firstWarnings, []) + await shutdown(firstCtx) + + // 2. Second boot where selectedDefault is already ops-ptc on disk, followed by a + // PluginManager.removeBundle-style two-phase HMR transaction under package.json.lock. + const secondRegistry = stubRegistry('ops-ptc') + const secondWarnings = [] + const secondCtx = new Context() + secondCtx.logger.level = 0 + secondCtx.provide('profile', { dir: profileDir }) + secondCtx.provide('hmr', hmr) + secondCtx.provide('agentPresets', secondRegistry) + secondCtx.provide('settings', makeSettings(secondRegistry)) + registerOpsPtcPreset(secondCtx, { warn: message => secondWarnings.push(message) }) + await settle(secondRegistry) + assert.deepEqual(secondRegistry.state.updates, [], 'second boot with ops-ptc already selected does not rewrite on startup') + + events.length = 0 + fs.writeFileSync(lockFile, `${process.pid}\n`) + try { + await hmr.runExclusive(async () => { + events.push('phase1:unload') + await secondCtx.fiber.dispose() + }) + // Simulate `pnpm remove dsh-ops` between the two HMR phases while `package.json.lock` stays held. + await new Promise(resolve => setTimeout(resolve, 25)) + events.push('between-phases') + await hmr.runExclusive(async () => { + events.push('phase2:reload') + }) + } finally { + fs.rmSync(lockFile, { force: true }) + } + + await waitFor(() => secondRegistry.state.selected === 'standard', { what: 'the restored official default after removeBundle' }) + assert.deepEqual(secondWarnings, []) + assert.deepEqual(events, ['phase1:unload', 'between-phases', 'phase2:reload', 'settings:standard']) + await shutdown(secondCtx) + }) +}) + await test('the host entry anchor resolves app-boot when the install does not hoist it', async () => { await withTempDir('preset-registration-layout', async dir => { const registerFile = installedDshOps(dir) diff --git a/test/profile-temp.test.mjs b/test/profile-temp.test.mjs index 7606aae5..3efe12f8 100644 --- a/test/profile-temp.test.mjs +++ b/test/profile-temp.test.mjs @@ -2,6 +2,7 @@ import fs from 'node:fs' import path from 'node:path' import { assert, test, report, withTempDir } from './lib/harness.mjs' import { profileCommand } from '../lib/profile-install.js' +import { findDanglingDefault, repairDanglingDefault } from '../lib/profile-default-repair.js' import { ensureSessionTemp, sessionTempDir } from '../lib/session-temp.js' const capture = body => { @@ -257,4 +258,62 @@ await test('the launcher receives an install spec verbatim, and the cmd line ref } }) }) +await test('uninstall repairs a dangling selectedDefault: ops-ptc in cordis.patch.yml (#881)', async () => { + await profileFixture(({ root, receipt }) => { + const profileDir = path.join(root, 'profiles', 'dsh-tui') + const patchFile = path.join(profileDir, 'cordis.patch.yml') + + // Only item in patch -> replaced with empty sequence `[]` behind comments. + fs.writeFileSync(patchFile, [ + '# Managed by DSH', + '- id: agent-preset-registry', + ' config:', + ' selectedDefault: ops-ptc', + '', + ].join('\n')) + assert.equal(capture(() => profileCommand('uninstall', { profile: 'tui', dryRun: true })).status, 0) + assert.match(fs.readFileSync(patchFile, 'utf8'), /selectedDefault: ops-ptc/, 'dry-run must not mutate cordis.patch.yml') + + process.env.PROFILE_TEST_EXIT = '9' + assert.equal(capture(() => profileCommand('uninstall', { profile: 'tui' })).status, 9) + assert.match(fs.readFileSync(patchFile, 'utf8'), /selectedDefault: ops-ptc/, 'a failed CLI removal must not touch cordis.patch.yml') + delete process.env.PROFILE_TEST_EXIT + fs.unlinkSync(receipt) + + assert.equal(capture(() => profileCommand('uninstall', { profile: 'tui' })).status, 0) + assert.equal(fs.readFileSync(patchFile, 'utf8'), '# Managed by DSH\n[]\n') + + // Sibling config keys and sibling rows survive, while a non-ops-ptc selection is untouched. + const withSiblings = [ + '- id: agent-preset-registry', + ' config:', + ' default: standard', + ' selectedDefault: "ops-ptc"', + '- id: other-plugin', + ' disabled: true', + '', + ].join('\n') + fs.writeFileSync(patchFile, withSiblings) + assert.deepEqual(findDanglingDefault(withSiblings), [3]) + assert.deepEqual(repairDanglingDefault(profileDir), { file: patchFile, dangling: true, changed: true }) + assert.equal(fs.readFileSync(patchFile, 'utf8'), [ + '- id: agent-preset-registry', + ' config:', + ' default: standard', + '- id: other-plugin', + ' disabled: true', + '', + ].join('\n')) + + const operatorChoice = [ + '- id: agent-preset-registry', + ' config:', + ' selectedDefault: ptc', + '', + ].join('\n') + fs.writeFileSync(patchFile, operatorChoice) + assert.deepEqual(repairDanglingDefault(profileDir), { file: patchFile, dangling: false, changed: false }) + assert.equal(fs.readFileSync(patchFile, 'utf8'), operatorChoice) + }) +}) report('profile-temp')