Skip to content

Commit 0889fd5

Browse files
authored
fix: Fixed cross-platform issues (#18)
* fix: Fixed cross-platform issues * code review fixes
1 parent 8ed66f9 commit 0889fd5

27 files changed

Lines changed: 225 additions & 80 deletions
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<!-- version-type: patch -->
2+
# frontend
3+
4+
## 🧪 Tests
5+
6+
- 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.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<!-- version-type: patch -->
2+
# service
3+
4+
## 🐛 Bug Fixes
5+
6+
### Yarn prerequisite check now resolves on Windows
7+
8+
`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.
9+
10+
### Graceful service shutdown on Windows
11+
12+
`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.
13+
14+
## 🧪 Tests
15+
16+
- Added a Windows-specific `killProcessGroup` spec asserting `taskkill` is invoked **without** `/F` for `SIGTERM` and **with** `/F` for `SIGKILL`.
17+
- 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.
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<!-- version-type: patch -->
2+
# stack-craft
3+
4+
## 🧪 Tests
5+
6+
- 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.

.yarn/versions/b4e99528.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
releases:
2+
frontend: patch
3+
service: patch
4+
stack-craft: patch

e2e/dogfooding.spec.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { randomBytes } from 'crypto'
2+
import { tmpdir } from 'os'
3+
import { join } from 'path'
24

35
import { expect, test } from '@playwright/test'
46

@@ -48,7 +50,7 @@ It is used to test the following features:
4850

4951
const dogfoodingPort = await getAvailablePort()
5052
const mcpPort = await getAvailablePort()
51-
const workingDirectory = `/tmp/e2e-dog-fooding-time-${uuid}`
53+
const workingDirectory = join(tmpdir(), `e2e-dog-fooding-time-${uuid}`)
5254

5355
await page.goto('/')
5456
await login(page)
@@ -59,7 +61,7 @@ It is used to test the following features:
5961
name: stackName,
6062
displayName,
6163
description,
62-
mainDirectory: '/tmp/e2e-test',
64+
mainDirectory: join(tmpdir(), 'e2e-test'),
6365
})
6466
})
6567

e2e/smoke.spec.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { expect, test } from '@playwright/test'
2+
import { tmpdir } from 'os'
3+
import { join } from 'path'
24

35
import {
46
addRepository,
@@ -15,7 +17,7 @@ test('App Flow', async ({ page, browserName }) => {
1517

1618
const stackName = `e2e-test-stack-${uuid}`
1719
const displayName = `E2E Test Stack - ${browserName} - ${uuid}`
18-
const workingDirectory = `/tmp/e2e-test-stack-${uuid}`
20+
const workingDirectory = join(tmpdir(), `e2e-test-stack-${uuid}`)
1921

2022
await page.goto('/')
2123
await login(page)
@@ -26,7 +28,7 @@ test('App Flow', async ({ page, browserName }) => {
2628
name: stackName,
2729
displayName,
2830
description: 'Created by E2E test',
29-
mainDirectory: '/tmp/e2e-test',
31+
mainDirectory: join(tmpdir(), 'e2e-test'),
3032
})
3133
})
3234

frontend/src/components/entity-forms/stack-form.spec.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1+
import { tmpdir } from 'os'
12
import { describe, expect, it, vi } from 'vitest'
23

34
vi.mock('../app-routes.js', () => ({}))
45

56
import { isStackFormPayload } from './stack-form.js'
67

8+
const TMP = tmpdir()
9+
710
describe('isStackFormPayload', () => {
811
const validPayload = {
912
name: 'my-stack',
@@ -17,7 +20,7 @@ describe('isStackFormPayload', () => {
1720
})
1821

1922
it('should accept payload without description (not validated)', () => {
20-
expect(isStackFormPayload({ name: 'x', displayName: 'X', mainDirectory: '/tmp' })).toBe(true)
23+
expect(isStackFormPayload({ name: 'x', displayName: 'X', mainDirectory: TMP })).toBe(true)
2124
})
2225

2326
it('should throw on null (no null guard)', () => {
@@ -29,15 +32,15 @@ describe('isStackFormPayload', () => {
2932
})
3033

3134
it('should reject when name is missing', () => {
32-
expect(isStackFormPayload({ displayName: 'X', mainDirectory: '/tmp' })).toBe(false)
35+
expect(isStackFormPayload({ displayName: 'X', mainDirectory: TMP })).toBe(false)
3336
})
3437

3538
it('should reject when name is empty', () => {
3639
expect(isStackFormPayload({ ...validPayload, name: '' })).toBe(false)
3740
})
3841

3942
it('should reject when displayName is missing', () => {
40-
expect(isStackFormPayload({ name: 'x', mainDirectory: '/tmp' })).toBe(false)
43+
expect(isStackFormPayload({ name: 'x', mainDirectory: TMP })).toBe(false)
4144
})
4245

4346
it('should reject when displayName is empty', () => {

frontend/src/pages/import-export/import-stack.spec.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { tmpdir } from 'os'
2+
import { join } from 'path'
13
import { describe, expect, it, vi } from 'vitest'
24

35
vi.mock('../../components/app-routes.js', () => ({}))
@@ -12,7 +14,7 @@ describe('isImportConfigPayload', () => {
1214
it('should accept payload with additional env fields', () => {
1315
expect(
1416
isImportConfigPayload({
15-
mainDirectory: '/tmp/stack',
17+
mainDirectory: join(tmpdir(), 'stack'),
1618
autoSetup: 'on',
1719
envSource_GITHUB_TOKEN: 'inherit',
1820
envValue_GITHUB_TOKEN: '',

service/src/app-models/prerequisites/actions/check-prerequisite-action.spec.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { usingAsync } from '@furystack/utils'
66
import { Prerequisite, PrerequisiteCheckResult, StackConfig } from 'common'
77
import type { PrerequisiteType } from 'common'
88
import { randomBytes } from 'crypto'
9+
import { tmpdir } from 'os'
910
import { describe, expect, it, vi } from 'vitest'
1011

1112
import { runCheck, CheckPrerequisiteAction } from './check-prerequisite-action.js'
@@ -89,6 +90,36 @@ describe('CheckPrerequisiteAction', () => {
8990
const result = await runCheck('yarn', { minimumVersion: '4.0.0' })
9091
expect(result.satisfied).toBe(false)
9192
})
93+
94+
it('should invoke `yarn --version` directly on POSIX', async () => {
95+
const originalPlatform = process.platform
96+
Object.defineProperty(process, 'platform', { value: 'linux', writable: true })
97+
try {
98+
execFileMock.mockResolvedValue({ stdout: '4.6.0\n', stderr: '' })
99+
await runCheck('yarn', { minimumVersion: '4.0.0' })
100+
expect(execFileMock).toHaveBeenCalledWith('yarn', ['--version'], expect.objectContaining({ timeout: 30_000 }))
101+
} finally {
102+
Object.defineProperty(process, 'platform', { value: originalPlatform, writable: true })
103+
}
104+
})
105+
106+
// Yarn ships as `yarn.cmd` / `yarn.ps1` on Windows; routing through `cmd.exe`
107+
// is the load-bearing fix that lets PATHEXT resolve the shim. Guard against regression.
108+
it('should route through cmd.exe on Windows to resolve the yarn shim', async () => {
109+
const originalPlatform = process.platform
110+
Object.defineProperty(process, 'platform', { value: 'win32', writable: true })
111+
try {
112+
execFileMock.mockResolvedValue({ stdout: '4.6.0\n', stderr: '' })
113+
await runCheck('yarn', { minimumVersion: '4.0.0' })
114+
expect(execFileMock).toHaveBeenCalledWith(
115+
'cmd.exe',
116+
['/c', 'yarn', '--version'],
117+
expect.objectContaining({ timeout: 30_000 }),
118+
)
119+
} finally {
120+
Object.defineProperty(process, 'platform', { value: originalPlatform, writable: true })
121+
}
122+
})
92123
})
93124

94125
describe('git', () => {
@@ -264,7 +295,7 @@ describe('CheckPrerequisiteAction', () => {
264295
})
265296
await stackConfigStore.add({
266297
stackName: 'test-stack',
267-
mainDirectory: '/tmp',
298+
mainDirectory: tmpdir(),
268299
environmentVariables: {
269300
STACK_CONFIGURED_VAR_12345: { source: 'custom', customValue: 'configured-value' },
270301
},

service/src/app-models/prerequisites/actions/check-prerequisite-action.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,12 @@ const checkNode = async (config: { minimumVersion: string }): Promise<CheckResul
5757
}
5858

5959
const checkYarn = async (config: { minimumVersion: string }): Promise<CheckResult> => {
60-
const { stdout } = await execFileAsync('yarn', ['--version'], { timeout: COMMAND_TIMEOUT })
60+
// Yarn ships as `yarn.cmd` / `yarn.ps1` shims on Windows, which `execFile` cannot resolve directly.
61+
// Route through cmd.exe so PATHEXT applies. POSIX uses execFile directly to avoid an extra fork.
62+
const isWindows = process.platform === 'win32'
63+
const { stdout } = isWindows
64+
? await execFileAsync('cmd.exe', ['/c', 'yarn', '--version'], { timeout: COMMAND_TIMEOUT })
65+
: await execFileAsync('yarn', ['--version'], { timeout: COMMAND_TIMEOUT })
6166
const version = extractVersion(stdout.trim())
6267
if (!version) {
6368
return { satisfied: false, output: `Could not parse Yarn version from: ${stdout.trim()}` }

0 commit comments

Comments
 (0)