diff --git a/.yarn/changelogs/frontend.b4e99528.md b/.yarn/changelogs/frontend.b4e99528.md new file mode 100644 index 0000000..f217ab0 --- /dev/null +++ b/.yarn/changelogs/frontend.b4e99528.md @@ -0,0 +1,6 @@ + +# frontend + +## ๐Ÿงช Tests + +- Replaced hard-coded `/tmp/...` paths in the payload-validator specs (`stack-form.spec.ts`, `import-stack.spec.ts`) with `os.tmpdir()`-based equivalents so the frontend `vitest` suite runs unmodified on Windows hosts (where `/tmp` does not exist) in addition to Linux and macOS. diff --git a/.yarn/changelogs/service.b4e99528.md b/.yarn/changelogs/service.b4e99528.md new file mode 100644 index 0000000..4451b36 --- /dev/null +++ b/.yarn/changelogs/service.b4e99528.md @@ -0,0 +1,17 @@ + +# service + +## ๐Ÿ› Bug Fixes + +### Yarn prerequisite check now resolves on Windows + +`checkYarn` previously called `execFile('yarn', ['--version'])`, which fails on Windows because Yarn is installed as a `yarn.cmd` / `yarn.ps1` shim โ€” `execFile` does not consult `PATHEXT` and therefore reports the prerequisite as "not matched". On Windows the call is now routed through `cmd.exe /c yarn --version` so the shim is resolved like it is in any interactive shell. POSIX still uses `execFile` directly. + +### Graceful service shutdown on Windows + +`ProcessRunner.killProcessGroup` always invoked `taskkill /pid โ€ฆ /T /F` on Windows, regardless of the requested signal. Because `/F` is an unconditional force-kill, the SIGTERM-then-wait-then-SIGKILL escalation in `ServiceLifecycleManager` collapsed into a single force kill โ€” managed services never received a chance to flush state on shutdown. The Windows branch now drops `/F` for non-`SIGKILL` signals and reserves it for `SIGKILL`, restoring the graceful-then-force escalation that POSIX already had. + +## ๐Ÿงช Tests + +- Added a Windows-specific `killProcessGroup` spec asserting `taskkill` is invoked **without** `/F` for `SIGTERM` and **with** `/F` for `SIGKILL`. +- Replaced hard-coded `/tmp/...` literals in service specs with `os.tmpdir()`-based paths (`join(tmpdir(), '...')`) so spec runs are portable across Linux, macOS, and Windows. Affected specs: `git-operations-service`, `git-watcher`, `git-head-watcher`, `service-delete-branch-action`, `import-export-actions`, `create-stack-action`, `service-branches-action`, `resolve-service-cwd`, `encrypt-existing-secrets`, `service-pipeline-orchestrator`, `service-env-resolver`, `stale-state-reconciler`, `service-file-manager`, `service-checkout-action`, `check-prerequisite-action`, `evaluate-prerequisites`. The two `/tmp` references that remain in `process-runner.spec.ts` are intentional โ€” they pair with explicit `process.platform = 'linux'` mocks that exercise the POSIX `'/bin/sh'` branch. diff --git a/.yarn/changelogs/stack-craft.b4e99528.md b/.yarn/changelogs/stack-craft.b4e99528.md new file mode 100644 index 0000000..efb98f6 --- /dev/null +++ b/.yarn/changelogs/stack-craft.b4e99528.md @@ -0,0 +1,6 @@ + +# stack-craft + +## ๐Ÿงช Tests + +- Replaced hard-coded `/tmp/...` paths in Playwright E2E specs (`e2e/smoke.spec.ts`, `e2e/dogfooding.spec.ts`) and the frontend payload-validator specs (`stack-form.spec.ts`, `import-stack.spec.ts`) with `os.tmpdir()`-based paths so the suite runs unmodified on Windows hosts (where `/tmp` does not exist) in addition to Linux and macOS. diff --git a/.yarn/versions/b4e99528.yml b/.yarn/versions/b4e99528.yml new file mode 100644 index 0000000..9f6b52f --- /dev/null +++ b/.yarn/versions/b4e99528.yml @@ -0,0 +1,4 @@ +releases: + frontend: patch + service: patch + stack-craft: patch diff --git a/e2e/dogfooding.spec.ts b/e2e/dogfooding.spec.ts index f65992f..369d6ee 100644 --- a/e2e/dogfooding.spec.ts +++ b/e2e/dogfooding.spec.ts @@ -1,4 +1,6 @@ import { randomBytes } from 'crypto' +import { tmpdir } from 'os' +import { join } from 'path' import { expect, test } from '@playwright/test' @@ -48,7 +50,7 @@ It is used to test the following features: const dogfoodingPort = await getAvailablePort() const mcpPort = await getAvailablePort() - const workingDirectory = `/tmp/e2e-dog-fooding-time-${uuid}` + const workingDirectory = join(tmpdir(), `e2e-dog-fooding-time-${uuid}`) await page.goto('/') await login(page) @@ -59,7 +61,7 @@ It is used to test the following features: name: stackName, displayName, description, - mainDirectory: '/tmp/e2e-test', + mainDirectory: join(tmpdir(), 'e2e-test'), }) }) diff --git a/e2e/smoke.spec.ts b/e2e/smoke.spec.ts index b40d9b8..4e74628 100644 --- a/e2e/smoke.spec.ts +++ b/e2e/smoke.spec.ts @@ -1,4 +1,6 @@ import { expect, test } from '@playwright/test' +import { tmpdir } from 'os' +import { join } from 'path' import { addRepository, @@ -15,7 +17,7 @@ test('App Flow', async ({ page, browserName }) => { const stackName = `e2e-test-stack-${uuid}` const displayName = `E2E Test Stack - ${browserName} - ${uuid}` - const workingDirectory = `/tmp/e2e-test-stack-${uuid}` + const workingDirectory = join(tmpdir(), `e2e-test-stack-${uuid}`) await page.goto('/') await login(page) @@ -26,7 +28,7 @@ test('App Flow', async ({ page, browserName }) => { name: stackName, displayName, description: 'Created by E2E test', - mainDirectory: '/tmp/e2e-test', + mainDirectory: join(tmpdir(), 'e2e-test'), }) }) diff --git a/frontend/src/components/entity-forms/stack-form.spec.ts b/frontend/src/components/entity-forms/stack-form.spec.ts index e991102..dece248 100644 --- a/frontend/src/components/entity-forms/stack-form.spec.ts +++ b/frontend/src/components/entity-forms/stack-form.spec.ts @@ -1,9 +1,12 @@ +import { tmpdir } from 'os' import { describe, expect, it, vi } from 'vitest' vi.mock('../app-routes.js', () => ({})) import { isStackFormPayload } from './stack-form.js' +const TMP = tmpdir() + describe('isStackFormPayload', () => { const validPayload = { name: 'my-stack', @@ -17,7 +20,7 @@ describe('isStackFormPayload', () => { }) it('should accept payload without description (not validated)', () => { - expect(isStackFormPayload({ name: 'x', displayName: 'X', mainDirectory: '/tmp' })).toBe(true) + expect(isStackFormPayload({ name: 'x', displayName: 'X', mainDirectory: TMP })).toBe(true) }) it('should throw on null (no null guard)', () => { @@ -29,7 +32,7 @@ describe('isStackFormPayload', () => { }) it('should reject when name is missing', () => { - expect(isStackFormPayload({ displayName: 'X', mainDirectory: '/tmp' })).toBe(false) + expect(isStackFormPayload({ displayName: 'X', mainDirectory: TMP })).toBe(false) }) it('should reject when name is empty', () => { @@ -37,7 +40,7 @@ describe('isStackFormPayload', () => { }) it('should reject when displayName is missing', () => { - expect(isStackFormPayload({ name: 'x', mainDirectory: '/tmp' })).toBe(false) + expect(isStackFormPayload({ name: 'x', mainDirectory: TMP })).toBe(false) }) it('should reject when displayName is empty', () => { diff --git a/frontend/src/pages/import-export/import-stack.spec.ts b/frontend/src/pages/import-export/import-stack.spec.ts index 94baeb6..fe94266 100644 --- a/frontend/src/pages/import-export/import-stack.spec.ts +++ b/frontend/src/pages/import-export/import-stack.spec.ts @@ -1,3 +1,5 @@ +import { tmpdir } from 'os' +import { join } from 'path' import { describe, expect, it, vi } from 'vitest' vi.mock('../../components/app-routes.js', () => ({})) @@ -12,7 +14,7 @@ describe('isImportConfigPayload', () => { it('should accept payload with additional env fields', () => { expect( isImportConfigPayload({ - mainDirectory: '/tmp/stack', + mainDirectory: join(tmpdir(), 'stack'), autoSetup: 'on', envSource_GITHUB_TOKEN: 'inherit', envValue_GITHUB_TOKEN: '', diff --git a/service/src/app-models/prerequisites/actions/check-prerequisite-action.spec.ts b/service/src/app-models/prerequisites/actions/check-prerequisite-action.spec.ts index 173fd7a..40bc225 100644 --- a/service/src/app-models/prerequisites/actions/check-prerequisite-action.spec.ts +++ b/service/src/app-models/prerequisites/actions/check-prerequisite-action.spec.ts @@ -6,6 +6,7 @@ import { usingAsync } from '@furystack/utils' import { Prerequisite, PrerequisiteCheckResult, StackConfig } from 'common' import type { PrerequisiteType } from 'common' import { randomBytes } from 'crypto' +import { tmpdir } from 'os' import { describe, expect, it, vi } from 'vitest' import { runCheck, CheckPrerequisiteAction } from './check-prerequisite-action.js' @@ -89,6 +90,36 @@ describe('CheckPrerequisiteAction', () => { const result = await runCheck('yarn', { minimumVersion: '4.0.0' }) expect(result.satisfied).toBe(false) }) + + it('should invoke `yarn --version` directly on POSIX', async () => { + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { value: 'linux', writable: true }) + try { + execFileMock.mockResolvedValue({ stdout: '4.6.0\n', stderr: '' }) + await runCheck('yarn', { minimumVersion: '4.0.0' }) + expect(execFileMock).toHaveBeenCalledWith('yarn', ['--version'], expect.objectContaining({ timeout: 30_000 })) + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, writable: true }) + } + }) + + // Yarn ships as `yarn.cmd` / `yarn.ps1` on Windows; routing through `cmd.exe` + // is the load-bearing fix that lets PATHEXT resolve the shim. Guard against regression. + it('should route through cmd.exe on Windows to resolve the yarn shim', async () => { + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { value: 'win32', writable: true }) + try { + execFileMock.mockResolvedValue({ stdout: '4.6.0\n', stderr: '' }) + await runCheck('yarn', { minimumVersion: '4.0.0' }) + expect(execFileMock).toHaveBeenCalledWith( + 'cmd.exe', + ['/c', 'yarn', '--version'], + expect.objectContaining({ timeout: 30_000 }), + ) + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, writable: true }) + } + }) }) describe('git', () => { @@ -264,7 +295,7 @@ describe('CheckPrerequisiteAction', () => { }) await stackConfigStore.add({ stackName: 'test-stack', - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables: { STACK_CONFIGURED_VAR_12345: { source: 'custom', customValue: 'configured-value' }, }, diff --git a/service/src/app-models/prerequisites/actions/check-prerequisite-action.ts b/service/src/app-models/prerequisites/actions/check-prerequisite-action.ts index fac0aa0..f1025ca 100644 --- a/service/src/app-models/prerequisites/actions/check-prerequisite-action.ts +++ b/service/src/app-models/prerequisites/actions/check-prerequisite-action.ts @@ -57,7 +57,12 @@ const checkNode = async (config: { minimumVersion: string }): Promise => { - const { stdout } = await execFileAsync('yarn', ['--version'], { timeout: COMMAND_TIMEOUT }) + // Yarn ships as `yarn.cmd` / `yarn.ps1` shims on Windows, which `execFile` cannot resolve directly. + // Route through cmd.exe so PATHEXT applies. POSIX uses execFile directly to avoid an extra fork. + const isWindows = process.platform === 'win32' + const { stdout } = isWindows + ? await execFileAsync('cmd.exe', ['/c', 'yarn', '--version'], { timeout: COMMAND_TIMEOUT }) + : await execFileAsync('yarn', ['--version'], { timeout: COMMAND_TIMEOUT }) const version = extractVersion(stdout.trim()) if (!version) { return { satisfied: false, output: `Could not parse Yarn version from: ${stdout.trim()}` } diff --git a/service/src/app-models/prerequisites/evaluate-prerequisites.spec.ts b/service/src/app-models/prerequisites/evaluate-prerequisites.spec.ts index 8c97a15..f3c1b15 100644 --- a/service/src/app-models/prerequisites/evaluate-prerequisites.spec.ts +++ b/service/src/app-models/prerequisites/evaluate-prerequisites.spec.ts @@ -1,5 +1,7 @@ import { PrerequisiteCheckResultDataSet, PrerequisiteDataSet, StackConfigDataSet } from '../data-store/tokens.js' import { getDataSetFor } from '@furystack/repository' +import { tmpdir } from 'os' +import { join } from 'path' import { beforeEach, describe, expect, it, vi } from 'vitest' import { withTestInjector } from '../../test-helpers.js' @@ -131,7 +133,7 @@ describe('evaluatePrerequisites', () => { const ts = new Date().toISOString() await getDataSetFor(elevated, StackConfigDataSet).add(elevated, { stackName: 'my-stack', - mainDirectory: '/tmp/stack', + mainDirectory: join(tmpdir(), 'stack'), environmentVariables: { MY_VAR: { source: 'custom', customValue: 'secret' }, }, diff --git a/service/src/app-models/services/actions/service-branches-action.spec.ts b/service/src/app-models/services/actions/service-branches-action.spec.ts index 33b108b..7429731 100644 --- a/service/src/app-models/services/actions/service-branches-action.spec.ts +++ b/service/src/app-models/services/actions/service-branches-action.spec.ts @@ -93,7 +93,7 @@ describe('ServiceBranchesAction', () => { const repo = getRepository(injector) await repo.getDataSetFor(StackConfig, 'stackName').add(injector, { stackName: 'stack-1', - mainDirectory: '/tmp/stacks', + mainDirectory: join(tmpdir(), 'stacks'), environmentVariables: {}, createdAt: '', updatedAt: '', diff --git a/service/src/app-models/services/actions/service-checkout-action.spec.ts b/service/src/app-models/services/actions/service-checkout-action.spec.ts index 6b1f318..12f169a 100644 --- a/service/src/app-models/services/actions/service-checkout-action.spec.ts +++ b/service/src/app-models/services/actions/service-checkout-action.spec.ts @@ -1,4 +1,6 @@ import { RequestError } from '@furystack/rest' +import { tmpdir } from 'os' +import { join } from 'path' import { describe, expect, it, vi } from 'vitest' import type { Injector } from '@furystack/inject' @@ -13,7 +15,7 @@ const seedClonedService = async (elevated: Injector) => { const repo = getRepository(elevated) await repo.getDataSetFor((await import('common')).StackConfig, 'stackName').add(elevated, { stackName: 'stack-1', - mainDirectory: '/tmp/stacks', + mainDirectory: join(tmpdir(), 'stacks'), environmentVariables: {}, createdAt: '', updatedAt: '', diff --git a/service/src/app-models/services/actions/service-delete-branch-action.spec.ts b/service/src/app-models/services/actions/service-delete-branch-action.spec.ts index f9b0900..6b6734a 100644 --- a/service/src/app-models/services/actions/service-delete-branch-action.spec.ts +++ b/service/src/app-models/services/actions/service-delete-branch-action.spec.ts @@ -1,12 +1,16 @@ import { ServiceDefinitionDataSet, ServiceStatusDataSet } from '../../data-store/tokens.js' import { getDataSetFor } from '@furystack/repository' import type { ServiceStatus } from 'common' +import { tmpdir } from 'os' +import { join } from 'path' import { describe, expect, it, vi } from 'vitest' import { GitHeadWatcher } from '../../../services/git-head-watcher.js' import { GitService } from '../../../services/git-service.js' import { createMockActionContext, withTestInjector } from '../../../test-helpers.js' import { ServiceDeleteBranchAction } from './service-delete-branch-action.js' import '../../../test-shims.js' + +const REPO_DIR = join(tmpdir(), 'repo') const seed = async ( elevated: Parameters[0]>[0]['elevated'], status: Partial = {}, @@ -43,9 +47,11 @@ const mockGit = (overrides: Partial = {}) => ...overrides, }) as unknown as GitService -vi.mock('../../../utils/resolve-service-cwd.js', () => ({ - resolveServiceCwd: vi.fn().mockResolvedValue('/tmp/repo'), -})) +vi.mock('../../../utils/resolve-service-cwd.js', async () => { + const osMod = await import('os') + const pathMod = await import('path') + return { resolveServiceCwd: vi.fn().mockResolvedValue(pathMod.join(osMod.tmpdir(), 'repo')) } +}) describe('ServiceDeleteBranchAction', () => { it('deletes a local branch that is not currently checked out', () => @@ -64,7 +70,7 @@ describe('ServiceDeleteBranchAction', () => { }) const result = await ServiceDeleteBranchAction(ctx) expect(result.chunk).toMatchObject({ success: true, deleted: 'feature/old' }) - expect(git.deleteLocalBranch).toHaveBeenCalledWith('/tmp/repo', 'feature/old', false) + expect(git.deleteLocalBranch).toHaveBeenCalledWith(REPO_DIR, 'feature/old', false) expect(git.checkout).not.toHaveBeenCalled() })) @@ -84,8 +90,8 @@ describe('ServiceDeleteBranchAction', () => { body: { branch: 'feature/gone', force: true }, }) const result = await ServiceDeleteBranchAction(ctx) - expect(git.checkout).toHaveBeenCalledWith('/tmp/repo', 'main') - expect(git.deleteLocalBranch).toHaveBeenCalledWith('/tmp/repo', 'feature/gone', true) + expect(git.checkout).toHaveBeenCalledWith(REPO_DIR, 'main') + expect(git.deleteLocalBranch).toHaveBeenCalledWith(REPO_DIR, 'feature/gone', true) expect(result.chunk).toMatchObject({ success: true, deleted: 'feature/gone', switchedTo: 'main' }) })) @@ -137,7 +143,7 @@ describe('ServiceDeleteBranchAction', () => { body: { branch: 'feature/gone', switchTo: 'develop' }, }) await ServiceDeleteBranchAction(ctx) - expect(git.checkout).toHaveBeenCalledWith('/tmp/repo', 'develop') + expect(git.checkout).toHaveBeenCalledWith(REPO_DIR, 'develop') })) it('throws when service does not exist', () => diff --git a/service/src/app-models/stacks/actions/create-stack-action.spec.ts b/service/src/app-models/stacks/actions/create-stack-action.spec.ts index d161f2c..d6fe680 100644 --- a/service/src/app-models/stacks/actions/create-stack-action.spec.ts +++ b/service/src/app-models/stacks/actions/create-stack-action.spec.ts @@ -1,5 +1,7 @@ import { getDataSetFor } from '@furystack/repository' import { randomBytes } from 'crypto' +import { tmpdir } from 'os' +import { join } from 'path' import { beforeAll, describe, expect, it, vi } from 'vitest' import { StackConfigDataSet, StackDefinitionDataSet } from '../../data-store/tokens.js' @@ -8,11 +10,13 @@ import { CreateStackAction } from './create-stack-action.js' type CreateStackBody = Parameters[0] extends { getBody: () => Promise } ? B : never +const STACK_DIR = join(tmpdir(), 'my-stack') + const baseBody: CreateStackBody = { name: 'my-stack', displayName: 'My Stack', description: 'Example', - mainDirectory: '/tmp/my-stack', + mainDirectory: STACK_DIR, environmentVariables: {}, } @@ -36,7 +40,7 @@ describe('CreateStackAction', () => { const configs = await getDataSetFor(elevated, StackConfigDataSet).find(elevated, {}) expect(configs).toHaveLength(1) - expect(configs[0]?.mainDirectory).toBe('/tmp/my-stack') + expect(configs[0]?.mainDirectory).toBe(STACK_DIR) }) }) diff --git a/service/src/app-models/stacks/actions/import-export-actions.spec.ts b/service/src/app-models/stacks/actions/import-export-actions.spec.ts index 8bd9c67..190d35f 100644 --- a/service/src/app-models/stacks/actions/import-export-actions.spec.ts +++ b/service/src/app-models/stacks/actions/import-export-actions.spec.ts @@ -16,6 +16,8 @@ import { StackDefinition, } from 'common' import { randomBytes } from 'crypto' +import { tmpdir } from 'os' +import { join } from 'path' import { describe, expect, it } from 'vitest' import { ExportStackAction } from './export-stack-action.js' @@ -261,7 +263,7 @@ describe('Import/Export Stack Actions', () => { }, ], config: { - mainDirectory: '/tmp/imported', + mainDirectory: join(tmpdir(), 'imported'), }, } @@ -276,7 +278,7 @@ describe('Import/Export Stack Actions', () => { const stackConfigs = await stackConfigStore.find({}) expect(stackConfigs).toHaveLength(1) - expect(stackConfigs[0]?.mainDirectory).toBe('/tmp/imported') + expect(stackConfigs[0]?.mainDirectory).toBe(join(tmpdir(), 'imported')) const serviceDefs = await serviceDefStore.find({}) expect(serviceDefs).toHaveLength(1) @@ -323,7 +325,7 @@ describe('Import/Export Stack Actions', () => { repositories: [], prerequisites: [], config: { - mainDirectory: '/tmp/reset', + mainDirectory: join(tmpdir(), 'reset'), }, } @@ -373,7 +375,7 @@ describe('Import/Export Stack Actions', () => { repositories: [], prerequisites: [], config: { - mainDirectory: '/tmp/config-test', + mainDirectory: join(tmpdir(), 'config-test'), services: { 'cfg-svc-1': { autoFetchEnabled: true, @@ -441,7 +443,7 @@ describe('Import/Export Stack Actions', () => { repositories: [], prerequisites: [], config: { - mainDirectory: '/tmp/multi', + mainDirectory: join(tmpdir(), 'multi'), }, } @@ -490,7 +492,7 @@ describe('Import/Export Stack Actions', () => { services: [], repositories: [], prerequisites: [], - config: { mainDirectory: '/tmp/dup' }, + config: { mainDirectory: join(tmpdir(), 'dup') }, } const elevated = useSystemIdentityContext({ injector }) @@ -555,7 +557,7 @@ describe('Import/Export Stack Actions', () => { ], repositories: [], prerequisites: [], - config: { mainDirectory: '/tmp/conflict' }, + config: { mainDirectory: join(tmpdir(), 'conflict') }, } const elevated = useSystemIdentityContext({ injector }) @@ -603,7 +605,7 @@ describe('Import/Export Stack Actions', () => { }, ], prerequisites: [], - config: { mainDirectory: '/tmp/repo-conflict' }, + config: { mainDirectory: join(tmpdir(), 'repo-conflict') }, } const elevated = useSystemIdentityContext({ injector }) @@ -689,7 +691,7 @@ describe('Import/Export Stack Actions', () => { installationHelp: '', }, ], - config: { mainDirectory: '/tmp/multi-conflict' }, + config: { mainDirectory: join(tmpdir(), 'multi-conflict') }, } const elevated = useSystemIdentityContext({ injector }) @@ -733,7 +735,7 @@ describe('Import/Export Stack Actions', () => { repositories: [], prerequisites: [], config: { - mainDirectory: '/tmp/env-test', + mainDirectory: join(tmpdir(), 'env-test'), environmentVariables: { DATABASE_URL: { source: 'custom' as const, customValue: 'postgres://localhost/db' }, API_KEY: { source: 'inherit' as const }, diff --git a/service/src/services/git-head-watcher.spec.ts b/service/src/services/git-head-watcher.spec.ts index 434f8c2..fb30792 100644 --- a/service/src/services/git-head-watcher.spec.ts +++ b/service/src/services/git-head-watcher.spec.ts @@ -1,11 +1,17 @@ import { ServiceGitStatusDataSet } from '../app-models/data-store/tokens.js' import { getDataSetFor } from '@furystack/repository' +import { tmpdir } from 'os' +import { join } from 'path' import { beforeEach, describe, expect, it, vi } from 'vitest' import { withTestInjector } from '../test-helpers.js' import { GitHeadWatcher } from './git-head-watcher.js' import { GitService } from './git-service.js' import '../test-shims.js' + +const REPO_DIR = join(tmpdir(), 'repo') +const REPO_DIR_A = join(tmpdir(), 'a') +const REPO_DIR_B = join(tmpdir(), 'b') const { mockExistsSync, mockChokidarWatch, watcherFactory } = vi.hoisted(() => { const createWatcher = () => { const handlers = new Map void>>() @@ -60,7 +66,7 @@ describe('GitHeadWatcher', () => { injector.setExplicitInstance(mockGit as unknown as GitService, GitService) const headWatcher = injector.get(GitHeadWatcher) - await headWatcher.watch('svc-1', '/tmp/repo') + await headWatcher.watch('svc-1', REPO_DIR) expect(mockChokidarWatch).not.toHaveBeenCalled() expect(mockGit.getCurrentBranch).not.toHaveBeenCalled() @@ -77,9 +83,9 @@ describe('GitHeadWatcher', () => { injector.setExplicitInstance(mockGit as unknown as GitService, GitService) const headWatcher = injector.get(GitHeadWatcher) - await headWatcher.watch('svc-create', '/tmp/repo') + await headWatcher.watch('svc-create', REPO_DIR) - expect(mockGit.getCurrentBranch).toHaveBeenCalledWith('/tmp/repo') + expect(mockGit.getCurrentBranch).toHaveBeenCalledWith(REPO_DIR) const rows = await getDataSetFor(elevated, ServiceGitStatusDataSet).find(elevated, { filter: { serviceId: { $eq: 'svc-create' } }, }) @@ -98,7 +104,7 @@ describe('GitHeadWatcher', () => { injector.setExplicitInstance(mockGit as unknown as GitService, GitService) const headWatcher = injector.get(GitHeadWatcher) - await headWatcher.watch('svc-upd', '/tmp/repo') + await headWatcher.watch('svc-upd', REPO_DIR) const rows = await ds.find(elevated, { filter: { serviceId: { $eq: 'svc-upd' } } }) expect(rows).toHaveLength(1) @@ -120,8 +126,8 @@ describe('GitHeadWatcher', () => { const events: unknown[] = [] headWatcher.on('externalChange', (payload) => events.push(payload)) - await headWatcher.watch('svc-switch', '/tmp/repo') - createdWatcher.trigger('change', '/tmp/repo/.git/HEAD') + await headWatcher.watch('svc-switch', REPO_DIR) + createdWatcher.trigger('change', join(REPO_DIR, '.git', 'HEAD')) await vi.waitFor(() => expect(events).toHaveLength(1), { timeout: 500 }) expect(events[0]).toMatchObject({ @@ -147,8 +153,8 @@ describe('GitHeadWatcher', () => { const events: unknown[] = [] headWatcher.on('externalChange', (payload) => events.push(payload)) - await headWatcher.watch('svc-pull', '/tmp/repo') - createdWatcher.trigger('change', '/tmp/repo/.git/refs/heads/main') + await headWatcher.watch('svc-pull', REPO_DIR) + createdWatcher.trigger('change', join(REPO_DIR, '.git', 'refs', 'heads', 'main')) await vi.waitFor(() => expect(events).toHaveLength(1), { timeout: 500 }) expect(events[0]).toMatchObject({ serviceId: 'svc-pull', kind: 'pull-detected' }) @@ -164,7 +170,7 @@ describe('GitHeadWatcher', () => { mockChokidarWatch.mockReturnValueOnce(createdWatcher) const headWatcher = injector.get(GitHeadWatcher) - await headWatcher.watch('svc-unwatch', '/tmp/repo') + await headWatcher.watch('svc-unwatch', REPO_DIR) headWatcher.unwatch('svc-unwatch') expect(createdWatcher.close).toHaveBeenCalledTimes(1) @@ -190,8 +196,8 @@ describe('GitHeadWatcher', () => { mockChokidarWatch.mockReturnValueOnce(a).mockReturnValueOnce(b) const headWatcher = injector.get(GitHeadWatcher) - await headWatcher.watch('svc-a', '/tmp/a') - await headWatcher.watch('svc-b', '/tmp/b') + await headWatcher.watch('svc-a', REPO_DIR_A) + await headWatcher.watch('svc-b', REPO_DIR_B) await headWatcher[Symbol.asyncDispose]() expect(a.close).toHaveBeenCalledTimes(1) diff --git a/service/src/services/git-operations-service.spec.ts b/service/src/services/git-operations-service.spec.ts index 3b6fb18..6352a67 100644 --- a/service/src/services/git-operations-service.spec.ts +++ b/service/src/services/git-operations-service.spec.ts @@ -1,6 +1,8 @@ import type { Injector } from '@furystack/inject' import { GitHubRepository, ServiceConfig, ServiceDefinition, ServiceStatus, StackConfig } from 'common' import type fs from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' import { describe, expect, it, vi } from 'vitest' import { withTestInjector } from '../test-helpers.js' import { CryptoService } from '../utils/crypto-service.js' @@ -30,6 +32,7 @@ const existsSyncMock = vi.mocked(existsSync) const readdirSyncMock = vi.mocked(readdirSync) const ts = new Date().toISOString() +const TEST_STACK_DIR = join(tmpdir(), 'stacks', 'test') const setupMocks = (injector: Injector) => { const mockStatusManager = { updateServiceStatus: vi.fn().mockResolvedValue(undefined) } @@ -66,7 +69,7 @@ const seedServiceData = async ( const repo = getRepository(elevated) await repo.getDataSetFor(StackConfig, 'stackName').add(elevated, { stackName: 'test-stack', - mainDirectory: '/tmp/stacks/test', + mainDirectory: TEST_STACK_DIR, environmentVariables: {}, createdAt: ts, updatedAt: ts, @@ -127,7 +130,7 @@ describe('GitOperationsService', () => { expect(result).toEqual({ cloned: true, pulled: false, updated: true }) expect(mockGitService.clone).toHaveBeenCalledWith( 'https://github.com/furystack/stack-craft.git', - expect.stringContaining('/tmp/stacks/test'), + expect.stringContaining(TEST_STACK_DIR), ) expect(mockStatusManager.updateServiceStatus).toHaveBeenCalledWith( 'svc-1', diff --git a/service/src/services/git-watcher.spec.ts b/service/src/services/git-watcher.spec.ts index 903350f..34a344f 100644 --- a/service/src/services/git-watcher.spec.ts +++ b/service/src/services/git-watcher.spec.ts @@ -13,9 +13,13 @@ import { GitWatcher } from './git-watcher.js' import '../test-shims.js' const FETCH_CHECK_INTERVAL_MS = 5 * 60 * 1000 -vi.mock('../utils/resolve-service-cwd.js', () => ({ - resolveServiceCwd: vi.fn().mockResolvedValue('/tmp/repo'), -})) +vi.mock('../utils/resolve-service-cwd.js', async () => { + const { tmpdir } = await import('os') + const { join } = await import('path') + return { + resolveServiceCwd: vi.fn().mockResolvedValue(join(tmpdir(), 'repo')), + } +}) const addServiceDefinition = async (elevated: Injector, overrides: Partial = {}) => { await getDataSetFor(elevated, ServiceDefinitionDataSet).add(elevated, { diff --git a/service/src/services/process-runner.spec.ts b/service/src/services/process-runner.spec.ts index a0ac64f..52d1626 100644 --- a/service/src/services/process-runner.spec.ts +++ b/service/src/services/process-runner.spec.ts @@ -247,7 +247,7 @@ describe('ProcessRunner', () => { Object.defineProperty(process, 'platform', { value: originalPlatform, writable: true }) })) - it('should use taskkill on Windows', () => + it('should use taskkill without /F on Windows for graceful signals', () => withRunnerContext(async ({ runner }) => { const originalPlatform = process.platform Object.defineProperty(process, 'platform', { value: 'win32', writable: true }) @@ -257,6 +257,22 @@ describe('ProcessRunner', () => { const result = runner.killProcessGroup(child, 'SIGTERM') + expect(result).toBe(true) + expect(spawnSync).toHaveBeenCalledWith('taskkill', ['/pid', '999', '/T'], { stdio: 'ignore' }) + + Object.defineProperty(process, 'platform', { value: originalPlatform, writable: true }) + })) + + it('should use taskkill with /F on Windows for SIGKILL', () => + withRunnerContext(async ({ runner }) => { + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { value: 'win32', writable: true }) + + const { spawnSync } = await import('child_process') + const child = { pid: 999, kill: vi.fn() } as unknown as ChildProcess + + const result = runner.killProcessGroup(child, 'SIGKILL') + expect(result).toBe(true) expect(spawnSync).toHaveBeenCalledWith('taskkill', ['/pid', '999', '/T', '/F'], { stdio: 'ignore' }) diff --git a/service/src/services/process-runner.ts b/service/src/services/process-runner.ts index 1e105ac..d6b6fde 100644 --- a/service/src/services/process-runner.ts +++ b/service/src/services/process-runner.ts @@ -2,6 +2,7 @@ import { defineService, type Token } from '@furystack/inject' import { type ChildProcess, spawn, spawnSync } from 'child_process' import { LogStorageService } from './log-storage-service.js' +import type { ServiceLifecycleManager } from './service-lifecycle-manager.js' export type ManagedProcess = { serviceId: string @@ -113,12 +114,18 @@ export class ProcessRunnerImpl { /** * Kills a managed process and all its children by targeting the process group. * Falls back to killing just the shell process if the group kill fails. + * + * On Windows, `taskkill /T` (without `/F`) is used for non-`SIGKILL` signals so + * console apps that handle CTRL-C / WM_CLOSE get a chance to flush state. + * `/F` is reserved for `SIGKILL` to mirror the POSIX semantics that callers + * (e.g. {@link ServiceLifecycleManager}) rely on for the graceful-then-force escalation. */ public killProcessGroup(child: ChildProcess, signal: NodeJS.Signals): boolean { if (child.pid == null) return false try { if (process.platform === 'win32') { - spawnSync('taskkill', ['/pid', String(child.pid), '/T', '/F'], { stdio: 'ignore' }) + const args = signal === 'SIGKILL' ? ['/pid', String(child.pid), '/T', '/F'] : ['/pid', String(child.pid), '/T'] + spawnSync('taskkill', args, { stdio: 'ignore' }) } else { process.kill(-child.pid, signal) } diff --git a/service/src/services/service-env-resolver.spec.ts b/service/src/services/service-env-resolver.spec.ts index aa92205..da4760c 100644 --- a/service/src/services/service-env-resolver.spec.ts +++ b/service/src/services/service-env-resolver.spec.ts @@ -5,6 +5,7 @@ import { useLogging, VerboseConsoleLogger } from '@furystack/logging' import { usingAsync } from '@furystack/utils' import { Prerequisite, ServiceConfig, ServiceDefinition, ServicePrerequisiteLink, StackConfig } from 'common' import { randomBytes } from 'crypto' +import { tmpdir } from 'os' import { describe, expect, it } from 'vitest' import { CryptoService } from '../utils/crypto-service.js' @@ -136,7 +137,7 @@ describe('ServiceEnvResolver', () => { ) await stackConfigStore.add({ stackName: 'stack-b', - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables: { [varName]: { source: 'inherit' }, }, @@ -174,7 +175,7 @@ describe('ServiceEnvResolver', () => { ) await stackConfigStore.add({ stackName: 'stack-c', - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables: { MY_CUSTOM_VAR: { source: 'custom', customValue: 'plain-secret' }, }, @@ -208,7 +209,7 @@ describe('ServiceEnvResolver', () => { ) await stackConfigStore.add({ stackName: 'stack-d', - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables: { MY_ENCRYPTED_VAR: { source: 'custom', customValue: encrypted }, }, @@ -240,7 +241,7 @@ describe('ServiceEnvResolver', () => { ) await stackConfigStore.add({ stackName: 'stack-e', - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables: { OVERRIDDEN_VAR: { source: 'custom', customValue: 'stack-default' }, }, @@ -287,7 +288,7 @@ describe('ServiceEnvResolver', () => { ) await stackConfigStore.add({ stackName: 'stack-f', - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables: { [varName]: { source: 'inherit' }, }, @@ -305,7 +306,7 @@ describe('ServiceEnvResolver', () => { await svcDefStore.add(makeServiceDefinition({ id: 'svc-7', stackName: 'stack-g' })) await stackConfigStore.add({ stackName: 'stack-g', - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables: { FREE_FORM_VAR: { source: 'custom', customValue: 'stack-free-form' }, }, @@ -333,7 +334,7 @@ describe('ServiceEnvResolver', () => { await svcDefStore.add(makeServiceDefinition({ id: 'svc-8', stackName: 'stack-h' })) await stackConfigStore.add({ stackName: 'stack-h', - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables: {}, createdAt: ts, updatedAt: ts, @@ -370,7 +371,7 @@ describe('ServiceEnvResolver', () => { ) await stackConfigStore.add({ stackName: 'stack-i', - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables: { PREREQ_VAR: { source: 'custom', customValue: 'from-prereq' }, FREE_VAR: { source: 'custom', customValue: 'from-free-form' }, @@ -403,7 +404,7 @@ describe('ServiceEnvResolver', () => { await svcDefStore.add(makeServiceDefinition({ id: 'svc-10', stackName: 'stack-j' })) await stackConfigStore.add({ stackName: 'stack-j', - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables: { SHARED_FREE: { source: 'custom', customValue: 'stack-value' }, }, diff --git a/service/src/services/service-file-manager.spec.ts b/service/src/services/service-file-manager.spec.ts index 829e160..3ddfdad 100644 --- a/service/src/services/service-file-manager.spec.ts +++ b/service/src/services/service-file-manager.spec.ts @@ -1,5 +1,7 @@ import type { Injector } from '@furystack/inject' import { ServiceConfig, ServiceDefinition, ServiceStatus, StackConfig } from 'common' +import { tmpdir } from 'os' +import { join } from 'path' import { describe, expect, it, vi } from 'vitest' import { withTestInjector } from '../test-helpers.js' @@ -44,7 +46,7 @@ const seedData = async ( const repo = getRepository(elevated) await repo.getDataSetFor(StackConfig, 'stackName').add(elevated, { stackName: 'test-stack', - mainDirectory: '/tmp/stacks/test', + mainDirectory: join(tmpdir(), 'stacks', 'test'), environmentVariables: {}, createdAt: ts, updatedAt: ts, diff --git a/service/src/services/service-pipeline-orchestrator.spec.ts b/service/src/services/service-pipeline-orchestrator.spec.ts index 352b7c4..bfd5176 100644 --- a/service/src/services/service-pipeline-orchestrator.spec.ts +++ b/service/src/services/service-pipeline-orchestrator.spec.ts @@ -2,6 +2,8 @@ import { ServiceDependencyLinkDataSet, ServiceStatusDataSet } from '../app-model import { getDataSetFor } from '@furystack/repository' import type { Injector } from '@furystack/inject' import { ServiceDefinition, ServiceStatus, StackConfig } from 'common' +import { tmpdir } from 'os' +import { join } from 'path' import { describe, expect, it, vi } from 'vitest' import { withTestInjector } from '../test-helpers.js' import { GitOperationsService } from './git-operations-service.js' @@ -50,7 +52,7 @@ const seedService = async (elevated: Injector, overrides: Partial ({ - resolveServiceCwd: vi.fn().mockResolvedValue('/tmp/fake-cwd'), -})) + +const FAKE_CWD = join(tmpdir(), 'fake-cwd') + +vi.mock('../utils/resolve-service-cwd.js', async () => { + const { tmpdir: getTmpdir } = await import('os') + const { join: joinPath } = await import('path') + return { resolveServiceCwd: vi.fn().mockResolvedValue(joinPath(getTmpdir(), 'fake-cwd')) } +}) const setupMocks = (injector: Injector) => { const mockStatusManager = { updateServiceStatus: vi.fn().mockResolvedValue(undefined) } @@ -190,7 +197,7 @@ describe('StaleStateReconciler', () => { await reconciler.reconcileStaleStates() expect(vi.mocked(resolveServiceCwd)).toHaveBeenCalled() - expect(mockGitHeadWatcher.watch).toHaveBeenCalledWith('svc-cloned', '/tmp/fake-cwd') + expect(mockGitHeadWatcher.watch).toHaveBeenCalledWith('svc-cloned', FAKE_CWD) expect(mockGitWatcher.startWatching).toHaveBeenCalledWith('svc-cloned') })) diff --git a/service/src/utils/encrypt-existing-secrets.spec.ts b/service/src/utils/encrypt-existing-secrets.spec.ts index b6d04c1..6a7a7ce 100644 --- a/service/src/utils/encrypt-existing-secrets.spec.ts +++ b/service/src/utils/encrypt-existing-secrets.spec.ts @@ -3,6 +3,7 @@ import { getDataSetFor } from '@furystack/repository' import type { Injector } from '@furystack/inject' import type { ServiceConfig, StackConfig } from 'common' import { randomBytes } from 'crypto' +import { tmpdir } from 'os' import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { withTestInjector } from '../test-helpers.js' @@ -33,7 +34,7 @@ describe('encryptExistingSecrets', () => { ) => { await getDataSetFor(elevated, StackConfigDataSet).add(elevated, { stackName, - mainDirectory: '/tmp', + mainDirectory: tmpdir(), environmentVariables, createdAt: ts, updatedAt: ts, diff --git a/service/src/utils/resolve-service-cwd.spec.ts b/service/src/utils/resolve-service-cwd.spec.ts index f0eb266..df8b067 100644 --- a/service/src/utils/resolve-service-cwd.spec.ts +++ b/service/src/utils/resolve-service-cwd.spec.ts @@ -1,6 +1,7 @@ import { GitHubRepositoryDataSet, StackConfigDataSet } from '../app-models/data-store/tokens.js' import { getDataSetFor } from '@furystack/repository' -import { resolve } from 'path' +import { tmpdir } from 'os' +import { join, resolve } from 'path' import type { Injector } from '@furystack/inject' import type { ServiceDefinition } from 'common' import type { GitHubRepository } from 'common' @@ -8,6 +9,7 @@ import { describe, expect, it } from 'vitest' import { withTestInjector } from '../test-helpers.js' import { resolveServiceCwd } from './resolve-service-cwd.js' const ts = new Date().toISOString() +const STACKS_DIR = join(tmpdir(), 'stacks') describe('resolveServiceCwd', () => { const addStackConfig = async (elevated: Injector, stackName: string, mainDirectory: string) => { @@ -32,17 +34,17 @@ describe('resolveServiceCwd', () => { it('should resolve cwd from stack mainDirectory when no workingDirectory or repo', () => withTestInjector(async ({ injector, elevated }) => { - await addStackConfig(elevated, 'my-stack', '/tmp/stacks') + await addStackConfig(elevated, 'my-stack', STACKS_DIR) const service = { stackName: 'my-stack', id: 'svc-1' } as ServiceDefinition const result = await resolveServiceCwd(injector, service, elevated) - expect(result).toBe(resolve('/tmp/stacks')) + expect(result).toBe(resolve(STACKS_DIR)) })) it('should include service workingDirectory in the resolved path', () => withTestInjector(async ({ injector, elevated }) => { - await addStackConfig(elevated, 'my-stack', '/tmp/stacks') + await addStackConfig(elevated, 'my-stack', STACKS_DIR) const service = { stackName: 'my-stack', id: 'svc-2', @@ -51,12 +53,12 @@ describe('resolveServiceCwd', () => { const result = await resolveServiceCwd(injector, service, elevated) - expect(result).toBe(resolve('/tmp/stacks/services/frontend')) + expect(result).toBe(resolve(STACKS_DIR, 'services/frontend')) })) it('should append repo name when service has a repositoryId', () => withTestInjector(async ({ injector, elevated }) => { - await addStackConfig(elevated, 'my-stack', '/tmp/stacks') + await addStackConfig(elevated, 'my-stack', STACKS_DIR) await addRepo(elevated, 'repo-1', 'my-stack', 'https://github.com/user/my-repo') const service = { stackName: 'my-stack', @@ -66,12 +68,12 @@ describe('resolveServiceCwd', () => { const result = await resolveServiceCwd(injector, service, elevated) - expect(result).toBe(resolve('/tmp/stacks/my-repo')) + expect(result).toBe(resolve(STACKS_DIR, 'my-repo')) })) it('should append repo name after workingDirectory', () => withTestInjector(async ({ injector, elevated }) => { - await addStackConfig(elevated, 'my-stack', '/tmp/stacks') + await addStackConfig(elevated, 'my-stack', STACKS_DIR) await addRepo(elevated, 'repo-1', 'my-stack', 'https://github.com/user/my-repo.git') const service = { stackName: 'my-stack', @@ -82,7 +84,7 @@ describe('resolveServiceCwd', () => { const result = await resolveServiceCwd(injector, service, elevated) - expect(result).toBe(resolve('/tmp/stacks/apps/my-repo')) + expect(result).toBe(resolve(STACKS_DIR, 'apps/my-repo')) })) it('should throw when stack config is not found', () => @@ -96,17 +98,17 @@ describe('resolveServiceCwd', () => { it('should work without an existing elevated injector', () => withTestInjector(async ({ injector, elevated }) => { - await addStackConfig(elevated, 'my-stack', '/tmp/stacks') + await addStackConfig(elevated, 'my-stack', STACKS_DIR) const service = { stackName: 'my-stack', id: 'svc-6' } as ServiceDefinition const result = await resolveServiceCwd(injector, service) - expect(result).toBe(resolve('/tmp/stacks')) + expect(result).toBe(resolve(STACKS_DIR)) })) it('should not dispose the provided elevated injector', () => withTestInjector(async ({ injector, elevated }) => { - await addStackConfig(elevated, 'my-stack', '/tmp/stacks') + await addStackConfig(elevated, 'my-stack', STACKS_DIR) const service = { stackName: 'my-stack', id: 'svc-7' } as ServiceDefinition await resolveServiceCwd(injector, service, elevated) @@ -117,7 +119,7 @@ describe('resolveServiceCwd', () => { it('should ignore repositoryId when the repository does not exist', () => withTestInjector(async ({ injector, elevated }) => { - await addStackConfig(elevated, 'my-stack', '/tmp/stacks') + await addStackConfig(elevated, 'my-stack', STACKS_DIR) const service = { stackName: 'my-stack', id: 'svc-8', @@ -126,6 +128,6 @@ describe('resolveServiceCwd', () => { const result = await resolveServiceCwd(injector, service, elevated) - expect(result).toBe(resolve('/tmp/stacks')) + expect(result).toBe(resolve(STACKS_DIR)) })) })