From 22ce8d69a1abdbe7c7a8b90c662eac89ed0d1e20 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 15 Sep 2026 00:41:17 -0700 Subject: [PATCH] fix(lint): enable anti-slop/no-module-mocking (#20783) The rule rejects `vi.mock` / `vi.doMock` / `vi.unstable_mockModule` and the `jest` equivalents, on the argument that a test which rewrites the module graph asserts against a stand-in the production code never sees. It is already off for `**/*.test.{ts,tsx}`, `**/*.spec.{ts,tsx}`, `tests/**` and `**/__mocks__/**` via the existing override in config/oxlint-anti-slop.json; that override is unchanged here. What the rule actually catches is module mocking that has drifted out of a spec and into a first-party `.ts` support module, where nothing marks it as test-only. 73 violations at baseline, all of them in test-support code. 9 were relocated back into spec files the override already exempts; the remaining 64 sit in 10 files that are test-only but do not match the override globs, and carry a file-level disable naming the rule and the reason. Relocated: - terminal-hydration-store-test-bootstrap.ts: the sonner / sync-runtime-graph / pty-transport `vi.mock` calls moved into the two specs that import it (terminals-hydration-canonical-rows, terminals-hydration-canonical-pty-overlap). Vitest hoists `vi.mock` inside a test file, so registration is strictly earlier than the previous module-eval-time call; the bootstrap keeps only the preload API proxy. Both importers were updated. - ipc-events-ssh-authority-test-fixtures.ts: the 6 direct-ssh `vi.doMock` calls moved into useIpcEvents-agent-status-ssh-authority.test.ts as a local `stubDirectSshModules()` helper, which also de-duplicates the three copies the spec already had inline. The fixture now returns the store state and coordinator doubles it builds, typed via the exported DirectSshReconnectCoordinatorDouble. Suppressed, with justification (each is `/* oxlint-disable anti-slop/no-module-mocking -- ... */`, rule named, no blanket disable): - config/scripts/headless-serve-shutdown-matrix.test.mjs (1) - a genuine Vitest spec that the override misses only because its globs say {ts,tsx}. The script under test is a top-level CLI module; the alternative is spawning real docker. - src/main/codex-accounts/runtime-home-service-test-harness.ts (1) - stubs one probe predicate in ../pty/shell-startup-env, imported directly by several main-process readers; 17 specs share it. - src/main/computer/desktop-script-provider-test-harness.ts (2) - stubs child_process/fs-promises for a provider that shells out; 8 specs share it. - src/main/github/work-item-search-test-harness.ts (4) - one consumer lives in tests/e2e, where the relative mock ids resolve differently, so moving the calls into the specs would silently stop mocking there. - src/renderer/src/components/automations/automations-page-test-harness.tsx (14) - the mount rig for 10 AutomationsPage specs. - src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-test-harness.ts (1) - stubs refreshWebRuntimeSessionTabsSnapshot, imported directly by several renderer runtime modules; 18 specs share it. - src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures.ts (7) - stubReactSyncEffect/stubAuxiliaryModules, shared by 11 specs. - src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts (11) - stubs and hook invocation are one unit; 4 specs share it. - src/renderer/src/hooks/ipc-events-terminal-create-test-harness.ts (13) - its only spec is at 799 of an 800 max-lines budget. - src/renderer/src/hooks/ipc-events-test-harness.ts (10) - shared by 8 specs. No violation was converted to real dependency injection, and no max-lines disable was added. Verified: the audit command exits 0 with no output (and reports errors on a planted probe, so the rule is live); node config/scripts/run-typecheck-projects-in-parallel.mjs exits 0; 354 spec files / 2506 tests covering every importer of every touched file pass. No mobile/ file was touched. The changed-code quality gate's root Oxlint scan runs without --config so it never loads the anti-slop JS plugin, which made all 10 of those file-level suppressions read as "Unused oxlint-disable directive". check-changed-code-quality.mjs now exempts directives naming an anti-slop rule from that unused-directive warning, the same carve-out isCastingDirectiveUnusedWarning already makes for the casting suppressions the casting config enforces. Such a directive can never suppress a root-config rule, so nothing the root scan would otherwise report is hidden; audit:anti-slop remains the scan that enforces the rule. --- config/oxlint-anti-slop.json | 2 +- config/scripts/check-changed-code-quality.mjs | 17 +++ .../check-changed-code-quality.test.mjs | 44 +++++++ .../headless-serve-shutdown-matrix.test.mjs | 4 + .../runtime-home-service-test-harness.ts | 5 + .../desktop-script-provider-test-harness.ts | 3 + .../github/work-item-search-test-harness.ts | 3 + .../automations-page-test-harness.tsx | 3 + ...mote-runtime-pty-transport-test-harness.ts | 3 + ...vents-agent-status-window-test-fixtures.ts | 3 + .../ipc-events-close-routing-test-harness.ts | 3 + .../ipc-events-ssh-authority-test-fixtures.ts | 53 +++------ ...ipc-events-terminal-create-test-harness.ts | 3 + .../src/hooks/ipc-events-test-harness.ts | 3 + ...cEvents-agent-status-ssh-authority.test.ts | 112 ++++++++---------- ...terminal-hydration-store-test-bootstrap.ts | 16 +-- ...ls-hydration-canonical-pty-overlap.test.ts | 14 ++- ...terminals-hydration-canonical-rows.test.ts | 11 +- 18 files changed, 186 insertions(+), 116 deletions(-) diff --git a/config/oxlint-anti-slop.json b/config/oxlint-anti-slop.json index f0c779bd7e2..9e5eda11ae0 100644 --- a/config/oxlint-anti-slop.json +++ b/config/oxlint-anti-slop.json @@ -29,7 +29,7 @@ "anti-slop/no-chained-type-assertions": "off", "anti-slop/no-conditional-empty-object-spread": "off", "anti-slop/no-known-value-widening": "off", - "anti-slop/no-module-mocking": "off", + "anti-slop/no-module-mocking": "error", "anti-slop/no-object-parameters": "off", "anti-slop/no-reduce-accumulator-copy": "error", "anti-slop/no-reflect-apply": "error", diff --git a/config/scripts/check-changed-code-quality.mjs b/config/scripts/check-changed-code-quality.mjs index 31b6d24953a..1b8a0c4f5e9 100644 --- a/config/scripts/check-changed-code-quality.mjs +++ b/config/scripts/check-changed-code-quality.mjs @@ -11,6 +11,8 @@ const ROOT_CODE_QUALITY_IGNORED_PREFIXES = ['cloud/'] const CASTING_RULE = 'typescript/consistent-type-assertions' const CASTING_DISABLE_PATTERN = /\/[/*]\s*(?:oxlint|eslint)-disable(?:-next-line|-line)?\s[^\n]*typescript\/consistent-type-assertions/ +const ANTI_SLOP_DISABLE_PATTERN = + /\/[/*]\s*(?:oxlint|eslint)-disable(?:-next-line|-line)?\s[^\n]*\banti-slop\// export const OXLINT_SCANS = [ { // Why: no --config, so Oxlint keeps discovering nested configs. Pinning the root @@ -330,6 +332,20 @@ export function isCastingDirectiveUnusedWarning(diagnostic, root) { ) } +// Why: the anti-slop rules live in a JS plugin that only config/oxlint-anti-slop.json loads, so +// the root scan never sees those rule names and reports every anti-slop suppression as unused. +// `audit:anti-slop` is the scan that enforces them. +export function isAntiSlopDirectiveUnusedWarning(diagnostic, root) { + if (!/^Unused (?:oxlint|eslint)-disable/.test(diagnostic.message ?? '')) { + return false + } + return (diagnostic.labels ?? []).some((label) => + diagnosticHighlightedLines(root, diagnostic.filename, label.span).some((line) => + ANTI_SLOP_DISABLE_PATTERN.test(line) + ) + ) +} + // Why: oxlint cannot see the AGENTS.md requirement that every casting suppression carry a // line-specific SAFETY: rationale, so the directive text itself is checked over added lines. export function findCastingDirectivesMissingSafety(root, rangesByFile) { @@ -402,6 +418,7 @@ export function main( (diagnostic) => !isSuppressedDiagnostic(diagnostic, root) && !isCastingDirectiveUnusedWarning(diagnostic, root) && + !isAntiSlopDirectiveUnusedWarning(diagnostic, root) && diagnosticTouchesAddedLines(diagnostic, rangesByFile, root, baseBlocks) ) for (const diagnostic of diagnostics) { diff --git a/config/scripts/check-changed-code-quality.test.mjs b/config/scripts/check-changed-code-quality.test.mjs index 3a88cf1b02e..a0bfcd0ecd9 100644 --- a/config/scripts/check-changed-code-quality.test.mjs +++ b/config/scripts/check-changed-code-quality.test.mjs @@ -1,7 +1,10 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import path from 'node:path' import { describe, expect, it } from 'vitest' import { OXLINT_SCANS, diagnosticTouchesAddedLines, + isAntiSlopDirectiveUnusedWarning, isMovedCode, isRootCodeQualityPath, overlapsAddedLines, @@ -110,3 +113,44 @@ describe('moved-code exemption', () => { expect(isMovedCode(['', ' '], [['a()']])).toBe(false) }) }) + +describe('anti-slop directive unused warning', () => { + const root = path.resolve(import.meta.dirname, '..', '..') + // Assembled so no line here is itself a directive the gate would scan. + const directive = (rule) => `/* oxlint-disable ${rule} -- reason */` + + const withFixture = (firstLine, assert) => { + const directory = mkdtempSync(path.join(root, 'config', 'anti-slop-directive-test-')) + try { + const file = path.join(directory, 'fixture.ts') + writeFileSync(file, [firstLine, 'export const value = 1', ''].join('\n')) + assert({ + message: 'Unused oxlint-disable directive (no problems were reported).', + filename: file, + labels: [{ span: { line: 1 } }] + }) + } finally { + rmSync(directory, { recursive: true, force: true }) + } + } + + it('exempts a suppression the root scan cannot resolve', () => { + withFixture(directive('anti-slop/no-module-mocking'), (diagnostic) => { + expect(isAntiSlopDirectiveUnusedWarning(diagnostic, root)).toBe(true) + }) + }) + + it('still reports an unused directive for a rule the root scan does load', () => { + withFixture(directive('unicorn/no-array-reduce'), (diagnostic) => { + expect(isAntiSlopDirectiveUnusedWarning(diagnostic, root)).toBe(false) + }) + }) + + it('ignores diagnostics that are not unused-directive warnings', () => { + withFixture(directive('anti-slop/no-module-mocking'), (diagnostic) => { + expect( + isAntiSlopDirectiveUnusedWarning({ ...diagnostic, message: 'Unexpected any.' }, root) + ).toBe(false) + }) + }) +}) diff --git a/config/scripts/headless-serve-shutdown-matrix.test.mjs b/config/scripts/headless-serve-shutdown-matrix.test.mjs index 7231cc21e4f..32878b219f5 100644 --- a/config/scripts/headless-serve-shutdown-matrix.test.mjs +++ b/config/scripts/headless-serve-shutdown-matrix.test.mjs @@ -1,3 +1,7 @@ +/* oxlint-disable anti-slop/no-module-mocking -- This IS the Vitest spec for run-headless-serve-shutdown-docker.mjs, but the rule's test-file + override globs only .ts/.tsx, so a .test.mjs spec slips through. The script under test is a + top-level CLI module driven via vi.resetModules() + await import(); the only other way to observe + its docker argv is to spawn real docker. */ import { createHash } from 'node:crypto' import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' diff --git a/src/main/codex-accounts/runtime-home-service-test-harness.ts b/src/main/codex-accounts/runtime-home-service-test-harness.ts index 3922823ebd1..86d94de5807 100644 --- a/src/main/codex-accounts/runtime-home-service-test-harness.ts +++ b/src/main/codex-accounts/runtime-home-service-test-harness.ts @@ -1,3 +1,8 @@ +/* oxlint-disable anti-slop/no-module-mocking -- Vitest support module for the 17 runtime-home specs, not shipped code, and it falls outside the *.test / *.spec / tests glob set. + setupRuntimeHomeTest() overrides one probe predicate in ../pty/shell-startup-env; the production + readers import it directly across several main-process modules, so an injected seam would have to + be threaded through all of them. Inlining the stub into each of the 17 specs would duplicate it 17 + times and push the largest past the max-lines ratchet. */ import { expect, vi } from 'vitest' import { existsSync, diff --git a/src/main/computer/desktop-script-provider-test-harness.ts b/src/main/computer/desktop-script-provider-test-harness.ts index bcb0a4b0118..43d213c855f 100644 --- a/src/main/computer/desktop-script-provider-test-harness.ts +++ b/src/main/computer/desktop-script-provider-test-harness.ts @@ -1,3 +1,6 @@ +/* oxlint-disable anti-slop/no-module-mocking -- Vitest support module for the 8 desktop-script-provider specs, not shipped code, and it falls + outside the *.test / *.spec / tests glob set. The stubs replace node builtins (child_process, fs/promises) for a provider + that shells out; inlining them would duplicate the vi.hoisted fixture into all 8 specs. */ import { expect, vi } from 'vitest' import type { DesktopScriptRuntimeHost } from './desktop-script-runtime-host' diff --git a/src/main/github/work-item-search-test-harness.ts b/src/main/github/work-item-search-test-harness.ts index d3783f0afe1..d36732e9eb1 100644 --- a/src/main/github/work-item-search-test-harness.ts +++ b/src/main/github/work-item-search-test-harness.ts @@ -1,3 +1,6 @@ +/* oxlint-disable anti-slop/no-module-mocking -- Vitest support module for the 6 work-item-search specs, not shipped code, and it falls outside + the *.test / *.spec / tests glob set. One consumer lives in tests/e2e, where the relative mock ids ('../git/...') resolve to + different modules, so moving these calls into the specs would silently stop mocking there. */ import { afterEach, beforeEach, vi } from 'vitest' import type { Mock } from 'vitest' import { randomUUID } from 'node:crypto' diff --git a/src/renderer/src/components/automations/automations-page-test-harness.tsx b/src/renderer/src/components/automations/automations-page-test-harness.tsx index e34ae0640bc..fc94b173286 100644 --- a/src/renderer/src/components/automations/automations-page-test-harness.tsx +++ b/src/renderer/src/components/automations/automations-page-test-harness.tsx @@ -1,3 +1,6 @@ +/* oxlint-disable anti-slop/no-module-mocking -- Vitest support module for the 10 AutomationsPage specs, not shipped code, and it falls outside + the *.test / *.spec / tests glob set. Inlining these 13 stubs would duplicate them into all 10 specs and push the largest + past the max-lines ratchet. */ /** * The mount rig for AutomationsPage tests: child stand-ins, the preload API * double, and the per-test store reset. diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-test-harness.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-test-harness.ts index f5db3036f30..0cc62569b5f 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-test-harness.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-test-harness.ts @@ -1,3 +1,6 @@ +/* oxlint-disable anti-slop/no-module-mocking -- Vitest support module for the 18 remote-runtime PTY transport specs, not shipped code, and it + falls outside the *.test / *.spec / tests glob set. refreshWebRuntimeSessionTabsSnapshot is imported directly by several + renderer runtime modules, so an injected seam would have to be threaded through all of them. */ import { vi } from 'vitest' import type { Mock } from 'vitest' import { diff --git a/src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures.ts b/src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures.ts index e1b5f548996..1f58e87cb99 100644 --- a/src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures.ts +++ b/src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures.ts @@ -1,3 +1,6 @@ +/* oxlint-disable anti-slop/no-module-mocking -- Vitest support module for the 11 agent-status ipc-events specs, not shipped code, and it falls + outside the *.test / *.spec / tests glob set. Inlining stubReactSyncEffect and stubAuxiliaryModules would duplicate them + into all 11 specs and push several past the max-lines ratchet. */ import type * as ReactModule from 'react' import { vi } from 'vitest' import type { diff --git a/src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts b/src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts index 72e9683382c..3883f7ac8af 100644 --- a/src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts +++ b/src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts @@ -1,3 +1,6 @@ +/* oxlint-disable anti-slop/no-module-mocking -- Vitest support module for the 4 close-routing ipc-events specs, not shipped code, and it falls + outside the *.test / *.spec / tests glob set. The stubs and the hook invocation are one unit; splitting the 11 doMock + calls back out would duplicate them into all 4 specs. */ import type * as ReactModule from 'react' import { vi } from 'vitest' diff --git a/src/renderer/src/hooks/ipc-events-ssh-authority-test-fixtures.ts b/src/renderer/src/hooks/ipc-events-ssh-authority-test-fixtures.ts index 4c91e898a8b..9490b19a4e8 100644 --- a/src/renderer/src/hooks/ipc-events-ssh-authority-test-fixtures.ts +++ b/src/renderer/src/hooks/ipc-events-ssh-authority-test-fixtures.ts @@ -1,19 +1,29 @@ import { vi } from 'vitest' import { buildStoreState } from './ipc-events-agent-status-store-test-fixtures' -import { - buildWindowApi, - stubReactSyncEffect, - stubAuxiliaryModules -} from './ipc-events-agent-status-window-test-fixtures' +import type { StoreLike } from './ipc-events-agent-status-store-test-fixtures' +import { buildWindowApi } from './ipc-events-agent-status-window-test-fixtures' +export type DirectSshReconnectCoordinatorDouble = { + requestReconnect: ReturnType + replaceAuthority: ReturnType + prepareOnly: ReturnType + correctUnboundTerminals: ReturnType + finalizeHydratedTerminals: ReturnType + invalidate: ReturnType + stop: ReturnType +} + +/** Store/coordinator doubles for the partial-authority reconciliation path; the spec wires them. */ export function buildSshAuthorityReconciliationHarness(args: { partialAuthority: { providerEpoch?: string; connectionGeneration?: number } latestAuthority: { providerEpoch: string; connectionGeneration: number } }): { + coordinator: DirectSshReconnectCoordinatorDouble emitPartialState: () => void getState: ReturnType requestReconnect: ReturnType setSshConnectionState: ReturnType + storeState: StoreLike storedState: () => Record | undefined } { const targetId = 'target-reconciliation' @@ -55,37 +65,6 @@ export function buildSshAuthorityReconciliationHarness(args: { stop: vi.fn() } - stubReactSyncEffect() - stubAuxiliaryModules() - vi.doMock('../store', () => ({ - useAppStore: { - subscribe: vi.fn(() => () => {}), - getState: () => storeState - } - })) - vi.doMock('./direct-ssh-reconnect-rollout', () => ({ - isDirectSshReconnectCoordinatorRoutingEnabled: () => true - })) - vi.doMock('./direct-ssh-worktree-refresh-scheduler', () => ({ - createDirectSshWorktreeRefreshScheduler: () => ({ - stop: vi.fn(), - disposeProvider: vi.fn() - }) - })) - vi.doMock('./direct-ssh-host-hydration', () => ({ - createDirectSshHostHydration: () => ({ - capturePreparationInput: vi.fn(), - readHostScopedLineage: vi.fn(), - isPreparationTokenCurrent: vi.fn(() => true), - stop: vi.fn() - }) - })) - vi.doMock('./direct-ssh-reconnect-coordinator', () => ({ - createDirectSshReconnectCoordinator: () => coordinator - })) - vi.doMock('@/lib/direct-ssh-reconnect-product-telemetry', () => ({ - createDirectSshReconnectProductTelemetryAdapter: vi.fn() - })) vi.stubGlobal( 'window', buildWindowApi({ @@ -101,6 +80,7 @@ export function buildSshAuthorityReconciliationHarness(args: { ) return { + coordinator, emitPartialState: () => { if (!sshStateListener) { throw new Error('Expected SSH state listener') @@ -110,6 +90,7 @@ export function buildSshAuthorityReconciliationHarness(args: { getState, requestReconnect, setSshConnectionState, + storeState, storedState: () => sshConnectionStates.get(targetId) } } diff --git a/src/renderer/src/hooks/ipc-events-terminal-create-test-harness.ts b/src/renderer/src/hooks/ipc-events-terminal-create-test-harness.ts index b5c195e4a2d..0cad65374c0 100644 --- a/src/renderer/src/hooks/ipc-events-terminal-create-test-harness.ts +++ b/src/renderer/src/hooks/ipc-events-terminal-create-test-harness.ts @@ -1,3 +1,6 @@ +/* oxlint-disable anti-slop/no-module-mocking -- Vitest support module for useIpcEvents-terminal-create-surfacing.test.ts, not shipped code, and it + falls outside the *.test / *.spec / tests glob set. That spec already sits at 799 of its 800 max-lines budget, so these 13 + stubs cannot move back into it without a max-lines disable. */ import type * as ReactModule from 'react' import { vi } from 'vitest' import { buildTerminalCreateWindow } from './ipc-events-terminal-create-window-test-fixtures' diff --git a/src/renderer/src/hooks/ipc-events-test-harness.ts b/src/renderer/src/hooks/ipc-events-test-harness.ts index b3d1ef71b98..69ab58bf9d8 100644 --- a/src/renderer/src/hooks/ipc-events-test-harness.ts +++ b/src/renderer/src/hooks/ipc-events-test-harness.ts @@ -1,3 +1,6 @@ +/* oxlint-disable anti-slop/no-module-mocking -- Vitest support module for the 8 useIpcEvents specs, not shipped code, and it falls outside + the *.test / *.spec / tests glob set. Inlining these 10 stubs would duplicate them into all 8 specs and push the largest + past the max-lines ratchet. */ import { vi } from 'vitest' import type * as ReactModule from 'react' import type { HarnessStoreState } from './ipc-events-harness-store-state' diff --git a/src/renderer/src/hooks/useIpcEvents-agent-status-ssh-authority.test.ts b/src/renderer/src/hooks/useIpcEvents-agent-status-ssh-authority.test.ts index bc6ec581586..fc019448ac7 100644 --- a/src/renderer/src/hooks/useIpcEvents-agent-status-ssh-authority.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-agent-status-ssh-authority.test.ts @@ -1,11 +1,52 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { buildStoreState } from './ipc-events-agent-status-store-test-fixtures' +import type { StoreLike } from './ipc-events-agent-status-store-test-fixtures' import { buildWindowApi, stubReactSyncEffect, stubAuxiliaryModules } from './ipc-events-agent-status-window-test-fixtures' import { buildSshAuthorityReconciliationHarness } from './ipc-events-ssh-authority-test-fixtures' +import type { DirectSshReconnectCoordinatorDouble } from './ipc-events-ssh-authority-test-fixtures' + +function stubDirectSshModules(args: { + storeState: StoreLike + coordinator: DirectSshReconnectCoordinatorDouble + coordinatorRoutingEnabled?: boolean + capturePreparationInput?: ReturnType +}): void { + stubReactSyncEffect() + stubAuxiliaryModules() + vi.doMock('../store', () => ({ + useAppStore: { + subscribe: vi.fn(() => () => {}), + getState: () => args.storeState + } + })) + vi.doMock('./direct-ssh-reconnect-rollout', () => ({ + isDirectSshReconnectCoordinatorRoutingEnabled: () => args.coordinatorRoutingEnabled ?? true + })) + vi.doMock('./direct-ssh-worktree-refresh-scheduler', () => ({ + createDirectSshWorktreeRefreshScheduler: () => ({ + stop: vi.fn(), + disposeProvider: vi.fn() + }) + })) + vi.doMock('./direct-ssh-host-hydration', () => ({ + createDirectSshHostHydration: () => ({ + capturePreparationInput: args.capturePreparationInput ?? vi.fn(), + readHostScopedLineage: vi.fn(), + isPreparationTokenCurrent: vi.fn(() => true), + stop: vi.fn() + }) + })) + vi.doMock('./direct-ssh-reconnect-coordinator', () => ({ + createDirectSshReconnectCoordinator: () => args.coordinator + })) + vi.doMock('@/lib/direct-ssh-reconnect-product-telemetry', () => ({ + createDirectSshReconnectProductTelemetryAdapter: vi.fn() + })) +} // Why: end-to-end exercise of startup agent-status restoration through // useIpcEvents itself. The main process owns the durable cache; the renderer @@ -71,6 +112,7 @@ describe('useIpcEvents agent status snapshot integration', () => { connectionGeneration: 7 } }) + stubDirectSshModules({ storeState: harness.storeState, coordinator: harness.coordinator }) const { useIpcEvents } = await import('./useIpcEvents') useIpcEvents() @@ -105,6 +147,7 @@ describe('useIpcEvents agent status snapshot integration', () => { connectionGeneration: 7 } }) + stubDirectSshModules({ storeState: harness.storeState, coordinator: harness.coordinator }) const { useIpcEvents } = await import('./useIpcEvents') useIpcEvents() @@ -198,37 +241,12 @@ describe('useIpcEvents agent status snapshot integration', () => { } }) - stubReactSyncEffect() - stubAuxiliaryModules() - vi.doMock('../store', () => ({ - useAppStore: { - subscribe: vi.fn(() => () => {}), - getState: () => storeState - } - })) - vi.doMock('./direct-ssh-reconnect-rollout', () => ({ - isDirectSshReconnectCoordinatorRoutingEnabled: () => enabled - })) - vi.doMock('./direct-ssh-worktree-refresh-scheduler', () => ({ - createDirectSshWorktreeRefreshScheduler: () => ({ - stop: vi.fn(), - disposeProvider: vi.fn() - }) - })) - vi.doMock('./direct-ssh-host-hydration', () => ({ - createDirectSshHostHydration: () => ({ - capturePreparationInput, - readHostScopedLineage: vi.fn(), - isPreparationTokenCurrent: vi.fn(() => true), - stop: vi.fn() - }) - })) - vi.doMock('./direct-ssh-reconnect-coordinator', () => ({ - createDirectSshReconnectCoordinator: () => coordinator - })) - vi.doMock('@/lib/direct-ssh-reconnect-product-telemetry', () => ({ - createDirectSshReconnectProductTelemetryAdapter: vi.fn() - })) + stubDirectSshModules({ + storeState, + coordinator, + coordinatorRoutingEnabled: enabled, + capturePreparationInput + }) vi.stubGlobal( 'window', buildWindowApi({ @@ -429,37 +447,7 @@ describe('useIpcEvents agent status snapshot integration', () => { } let partialTargetStateCalls = 0 - stubReactSyncEffect() - stubAuxiliaryModules() - vi.doMock('../store', () => ({ - useAppStore: { - subscribe: vi.fn(() => () => {}), - getState: () => storeState - } - })) - vi.doMock('./direct-ssh-reconnect-rollout', () => ({ - isDirectSshReconnectCoordinatorRoutingEnabled: () => true - })) - vi.doMock('./direct-ssh-worktree-refresh-scheduler', () => ({ - createDirectSshWorktreeRefreshScheduler: () => ({ - stop: vi.fn(), - disposeProvider: vi.fn() - }) - })) - vi.doMock('./direct-ssh-host-hydration', () => ({ - createDirectSshHostHydration: () => ({ - capturePreparationInput: vi.fn(), - readHostScopedLineage: vi.fn(), - isPreparationTokenCurrent: vi.fn(() => true), - stop: vi.fn() - }) - })) - vi.doMock('./direct-ssh-reconnect-coordinator', () => ({ - createDirectSshReconnectCoordinator: () => coordinator - })) - vi.doMock('@/lib/direct-ssh-reconnect-product-telemetry', () => ({ - createDirectSshReconnectProductTelemetryAdapter: vi.fn() - })) + stubDirectSshModules({ storeState, coordinator }) vi.stubGlobal( 'window', buildWindowApi({ diff --git a/src/renderer/src/store/slices/terminal-hydration-store-test-bootstrap.ts b/src/renderer/src/store/slices/terminal-hydration-store-test-bootstrap.ts index ed47faad081..52aee154f55 100644 --- a/src/renderer/src/store/slices/terminal-hydration-store-test-bootstrap.ts +++ b/src/renderer/src/store/slices/terminal-hydration-store-test-bootstrap.ts @@ -1,16 +1,6 @@ -import { vi } from 'vitest' - -// Why: import this before the store modules — session hydration reaches for the preload API and the -// runtime/PTY singletons, which don't exist under vitest. -vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) -vi.mock('@/runtime/sync-runtime-graph', () => ({ - scheduleRuntimeGraphSync: vi.fn() -})) -vi.mock('@/components/terminal-pane/pty-transport', () => ({ - registerEagerPtyBuffer: vi.fn(), - ensurePtyDispatcher: vi.fn() -})) - +// Why: import this before the store modules — session hydration reaches for the preload API, which +// doesn't exist under vitest. Module stubs for sonner/runtime-graph/pty-transport live in the test +// files themselves so vitest can hoist them above the store imports. const apiProxy = (): unknown => new Proxy(() => undefined, { get: (_target, prop) => (prop === 'then' ? undefined : apiProxy()), diff --git a/src/renderer/src/store/slices/terminals-hydration-canonical-pty-overlap.test.ts b/src/renderer/src/store/slices/terminals-hydration-canonical-pty-overlap.test.ts index 170b1ceac81..63daa20874f 100644 --- a/src/renderer/src/store/slices/terminals-hydration-canonical-pty-overlap.test.ts +++ b/src/renderer/src/store/slices/terminals-hydration-canonical-pty-overlap.test.ts @@ -1,7 +1,6 @@ -// Keep this bare import first: its vi.mock calls run at module eval, and vitest only hoists vi.mock -// inside the test file itself — reordering it below the store imports breaks hydration here. +// Keep this bare import first: it installs the preload-API stub the store imports read at eval time. import './terminal-hydration-store-test-bootstrap' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { hydrateWorkspaceTerminalRows } from './terminal-session-row-hydration' import { getOrphanTerminalIds } from './terminal-orphan-helpers' import type { SleepingAgentSessionRecord } from '../../../../shared/agent-session-resume' @@ -12,6 +11,15 @@ import { getDefaultWorkspaceSession } from '../../../../shared/constants' import { buildWorkspaceSessionPayload } from '@/lib/workspace-session' import { createTestStore, makeLayout, makeTab, makeWorktree, seedStore } from './store-test-helpers' +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) +vi.mock('@/runtime/sync-runtime-graph', () => ({ + scheduleRuntimeGraphSync: vi.fn() +})) +vi.mock('@/components/terminal-pane/pty-transport', () => ({ + registerEagerPtyBuffer: vi.fn(), + ensurePtyDispatcher: vi.fn() +})) + const WORKTREE_ID = 'repo1::/wt-1' function makeCanonicalUnifiedTab(entityId: string, sortOrder: number): Tab { diff --git a/src/renderer/src/store/slices/terminals-hydration-canonical-rows.test.ts b/src/renderer/src/store/slices/terminals-hydration-canonical-rows.test.ts index 40eb541cc96..81362f8d50f 100644 --- a/src/renderer/src/store/slices/terminals-hydration-canonical-rows.test.ts +++ b/src/renderer/src/store/slices/terminals-hydration-canonical-rows.test.ts @@ -1,11 +1,20 @@ import './terminal-hydration-store-test-bootstrap' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { SleepingAgentSessionRecord } from '../../../../shared/agent-session-resume' import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' import { getDefaultWorkspaceSession } from '../../../../shared/constants' import { buildWorkspaceSessionPayload } from '@/lib/workspace-session' import { createTestStore, makeLayout, makeTab, makeWorktree, seedStore } from './store-test-helpers' +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) +vi.mock('@/runtime/sync-runtime-graph', () => ({ + scheduleRuntimeGraphSync: vi.fn() +})) +vi.mock('@/components/terminal-pane/pty-transport', () => ({ + registerEagerPtyBuffer: vi.fn(), + ensurePtyDispatcher: vi.fn() +})) + describe('hydrateWorkspaceSession canonical terminal rows', () => { it('drops only legacy rows that duplicate canonical PTY ownership', () => { const store = createTestStore()