From caf6cac3dfebdcb82fb1681c885da567ce0076fa Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:24:30 -0700 Subject: [PATCH] fix(codex): don't revoke a just-granted WSL home on a false 'missing' probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The async wsl.exe canonical-path settlement could report the runtime home 'missing' immediately after a verified RPC grant (a false negative — codex had just written and re-listed trust there), which drove the reconciliation 'remove' branch to delete all six granted [hooks.state] tables, leaving a bare [hooks.state] the launching pane read as 'hooks need review'. A 'missing' settlement now revokes only when no successful install ran this generation; a genuinely moved home still resolves to a different path and reinstalls. --- .../codex/hook-service-wsl-runtime.test.ts | 33 +++++++++++++++---- src/main/codex/hook-service.ts | 21 ++++++++++-- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/main/codex/hook-service-wsl-runtime.test.ts b/src/main/codex/hook-service-wsl-runtime.test.ts index c3d2ce9ca37..28c230ff160 100644 --- a/src/main/codex/hook-service-wsl-runtime.test.ts +++ b/src/main/codex/hook-service-wsl-runtime.test.ts @@ -209,7 +209,8 @@ describe('Codex WSL runtime hook install', () => { settlement: { status: 'unavailable' }, isCurrentGeneration: true, installedTrustConfigPath: '/mnt/d/home/hooks.json', - resolvedTrustConfigPath: null + resolvedTrustConfigPath: null, + installSucceeded: false }) ).toBe('none') @@ -218,7 +219,8 @@ describe('Codex WSL runtime hook install', () => { settlement: { status: 'missing' }, isCurrentGeneration: false, installedTrustConfigPath: '/mnt/d/home/hooks.json', - resolvedTrustConfigPath: null + resolvedTrustConfigPath: null, + installSucceeded: false }) ).toBe('none') @@ -227,25 +229,42 @@ describe('Codex WSL runtime hook install', () => { settlement: { status: 'missing' }, isCurrentGeneration: true, installedTrustConfigPath: '/mnt/d/home/hooks.json', - resolvedTrustConfigPath: null + resolvedTrustConfigPath: null, + installSucceeded: false }) ).toBe('remove') + // Why: a `missing` probe right after a verified grant is a false negative; + // revoking would delete the fresh trust the launching pane reads (#8847). expect( _internals.getWslHookReconciliationAction({ - settlement: { status: 'resolved', canonicalPath: '/windows/d/home' }, + settlement: { status: 'missing' }, isCurrentGeneration: true, - installedTrustConfigPath: '/windows/d/home/hooks.json', - resolvedTrustConfigPath: '/windows/d/home/hooks.json' + installedTrustConfigPath: '/mnt/d/home/hooks.json', + resolvedTrustConfigPath: null, + installSucceeded: true }) ).toBe('none') + expect( + _internals.getWslHookReconciliationAction({ + settlement: { status: 'resolved', canonicalPath: '/windows/d/home' }, + isCurrentGeneration: true, + installedTrustConfigPath: '/windows/d/home/hooks.json', + resolvedTrustConfigPath: '/windows/d/home/hooks.json', + installSucceeded: true + }) + ).toBe('none') + + // Why: a genuinely moved home resolves to a different path and still + // reinstalls, even though the original install succeeded. expect( _internals.getWslHookReconciliationAction({ settlement: { status: 'resolved', canonicalPath: '/windows/d/home' }, isCurrentGeneration: true, installedTrustConfigPath: '/mnt/d/home/hooks.json', - resolvedTrustConfigPath: '/windows/d/home/hooks.json' + resolvedTrustConfigPath: '/windows/d/home/hooks.json', + installSucceeded: true }) ).toBe('reinstall') }) diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index ab680f6edca..378d06174ed 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -930,12 +930,19 @@ function getWslHookReconciliationAction(args: { isCurrentGeneration: boolean installedTrustConfigPath: string | null resolvedTrustConfigPath: string | null + /** Whether the synchronous install for this generation wrote trust. */ + installSucceeded: boolean }): 'none' | 'remove' | 'reinstall' { if (!args.isCurrentGeneration) { return 'none' } if (args.settlement.status === 'missing') { - return 'remove' + // Why: a `missing` directory probe right after a verified install/grant is + // a false negative — the RPC (or fallback) just wrote and read trust in + // that home, so it exists. Revoking here would delete the fresh grant the + // launching pane needs, resurfacing "hooks need review". A genuinely moved + // home resolves to a different path and takes the `reinstall` branch below. + return args.installSucceeded ? 'none' : 'remove' } if ( args.settlement.status !== 'resolved' || @@ -972,6 +979,10 @@ export class CodexHookService { ): AgentHookInstallStatus | null { const generation = this.supersedeWslReconciliation(runtimeHomePath) let installedTrustConfigPath: string | null = null + // Why: JS is single-threaded, so the synchronous install below finishes + // before any async `wsl.exe` settlement callback runs — this flag is + // always set by the time the callback reads it. + let installSucceeded = false const onCanonicalPathSettled = (settlement: WslCanonicalPathSettlement): void => { if (!runtimeHomePath) { return @@ -989,7 +1000,8 @@ export class CodexHookService { settlement, isCurrentGeneration: this.wslReconciliationGeneration.get(key) === generation, installedTrustConfigPath, - resolvedTrustConfigPath: resolvedPlan?.trustConfigPath ?? null + resolvedTrustConfigPath: resolvedPlan?.trustConfigPath ?? null, + installSucceeded }) if (action === 'none') { return @@ -1014,6 +1026,7 @@ export class CodexHookService { return } installedTrustConfigPath = resolvedPlan.trustConfigPath + installSucceeded = status.state === 'installed' } const wslPlan = createCodexWslRuntimeHookInstallPlan( runtimeHomePath, @@ -1022,7 +1035,9 @@ export class CodexHookService { onCanonicalPathSettled ) installedTrustConfigPath = wslPlan?.trustConfigPath ?? null - return wslPlan ? installManagedHooksIntoWslRuntime(wslPlan) : null + const status = wslPlan ? installManagedHooksIntoWslRuntime(wslPlan) : null + installSucceeded = status?.state === 'installed' + return status } refreshRuntimeUserHooksForRuntimeHome(