mirror of
https://github.com/deepseek-ai/deepseek-harness.git
synced 2026-09-13 04:03:30 +00:00
Merge pull request #3781 from deepseek-harness/worktree/ci-reliability-master-20260908
test(ci): honor test budgets and clean up fixture processes
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write .agents/notes/implemented/testing/2026-09-08-ci-readiness-and-completion.md
|
||||
2026-09-08-ci-readiness-and-completion.md: 8ea0a5892d78eda16e657334dba2af58b6648d09
|
||||
2026-09-08-ci-readiness-and-completion.zh.md: 62a4c64081369a20a576805fb8a465bffff2922d
|
||||
@@ -0,0 +1,33 @@
|
||||
# Agent Note: CI assertions wait for owned completion
|
||||
|
||||
Status: implemented
|
||||
|
||||
English | [中文](2026-09-08-ci-readiness-and-completion.zh.md)
|
||||
|
||||
## Problem
|
||||
|
||||
The [empty master PR run](https://github.com/deepseek-harness/deepseek-harness/actions/runs/34206953049) fails while waiting one second for webhook Session creation and five seconds for PowerShell output. Neither test measures a startup latency guarantee. A [separate run](https://github.com/deepseek-harness/deepseek-harness/actions/runs/34207864157) shows the same short-budget problem in a desktop worker readiness test and captures a feedback acknowledgement while the composer still holds the submitted command.
|
||||
|
||||
Another [Windows coverage run](https://github.com/deepseek-harness/deepseek-harness/actions/runs/34224004885/job/102053583437) reports a null publint child status and an LSP initialization-marker timeout. Their helpers impose five- and three-second limits inside the lane's 90-second test budget. These cases verify publication contents and cancellation behavior rather than cold-start latency.
|
||||
|
||||
## Decision
|
||||
|
||||
The [webhook browser test](../../../../apps/web/tests/github-ready-review.e2e.ts) observes the model request caused by delivery before checking Session registration. The [feedback test](../../../../apps/web/tests/feedback-command.e2e.ts) waits for the empty composer and enabled attachment control before comparing ARIA output. Matching consecutive snapshots cannot prove that the command RPC has settled: its event stream can publish the acknowledgement first.
|
||||
|
||||
The [desktop transaction test](../../../../apps/desktop/tests/project-manager.spec.ts) gives the worker readiness marker the active test's execution budget. Its independent `afterEach` releases and awaits workers before deleting private roots, including when the runner abandons a timed-out test body. The poll observes runner cancellation, and teardown reports transaction failures independently from assertion failures. The [PowerShell tests](../../../../packages/shell/pwsh-local/tests/executor.spec.ts) register each helper-created Context before plugin initialization and dispose those Contexts before deleting temporary directories. The background-input case awaits process completion before checking complete output, completed status, and exit code. Consuming reads remain covered by their separate streaming tests.
|
||||
|
||||
The [publint runner tests](../../../../scripts/publint-all.spec.ts) pass the active test budget to their child and check launch errors and termination signals before interpreting its exit code. The [LSP instance test](../../../../packages/lsp/lsp-stdio/tests/instance.spec.ts) uses the same budget for its fixture marker, observes the actual pending `didOpen` write before aborting, and captures the query's rejection before waiting for readiness. Its [server fixture](../../../../packages/lsp/lsp-stdio/tests/fixture-server.ts) publishes the marker after pausing stdin. Teardown captures the instance list, Context, and directory before its first await.
|
||||
|
||||
The [subagent teardown decision](2026-09-07-subagent-teardown-test-budgets.md) owns lifecycle cleanup budgets. The [persistent PowerShell decision](2026-09-07-pwsh-ci-observable-completion.md) owns exact versus inferred terminal readiness; a one-shot process's completion promise has different semantics.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Larger independent waits.** Rejected where a completion promise already exists. A separate polling deadline continues to compete with the execution lane's budget.
|
||||
|
||||
**Refresh the feedback golden.** Rejected: the populated composer and disabled attachment control describe an in-flight submission. The settled expected UI remains the intended behavior.
|
||||
|
||||
**Serialize CI or retry these tests.** Rejected: neither establishes the missing completion condition or releases a blocked child after assertion failure.
|
||||
|
||||
## Consequences
|
||||
|
||||
Readiness and output assertions preserve their original content and ownership checks. Controlled desktop readiness, webhook preflight, command-response, PowerShell output, publint startup, and LSP initialization delays reproduce the original failures and pass with the completion waits. A stalled desktop-worker control still reports a test timeout while proving that teardown drains the child before removing its directory. The execution lane bounds test bodies and cleanup hooks separately; native Windows execution remains necessary to verify PowerShell and process cleanup there.
|
||||
@@ -0,0 +1,33 @@
|
||||
# Agent Note: CI 断言等待所属操作完成
|
||||
|
||||
Status: implemented
|
||||
|
||||
[English](2026-09-08-ci-readiness-and-completion.md) | 中文
|
||||
|
||||
## 问题
|
||||
|
||||
[master 空 PR 的运行](https://github.com/deepseek-harness/deepseek-harness/actions/runs/34206953049)在等待 Webhook Session 创建一秒、等待 PowerShell 输出五秒时失败。两个测试都不衡量启动延迟保证。[另一次运行](https://github.com/deepseek-harness/deepseek-harness/actions/runs/34207864157)在 Desktop worker 就绪测试中暴露了相同的局部短时限问题,并在输入框仍保留已提交命令时截取了反馈确认。
|
||||
|
||||
另一次 [Windows coverage 运行](https://github.com/deepseek-harness/deepseek-harness/actions/runs/34224004885/job/102053583437)报告了 publint 子进程退出状态为 null,以及 LSP 初始化标记等待超时。对应 helper 在通道的 90 秒测试预算内另设五秒和三秒限制。这些用例验证发布内容与取消行为,不衡量冷启动延迟。
|
||||
|
||||
## 决策
|
||||
|
||||
[Webhook 浏览器测试](../../../../apps/web/tests/github-ready-review.e2e.ts)观察投递触发的模型请求后再检查 Session 注册。[反馈测试](../../../../apps/web/tests/feedback-command.e2e.ts)在比较 ARIA 输出前等待输入框清空且附件按钮启用。连续两次快照相同不能证明命令 RPC 已完成:事件流可能先发布确认消息。
|
||||
|
||||
[Desktop 事务测试](../../../../apps/desktop/tests/project-manager.spec.ts)为 worker 就绪标记使用当前测试的执行预算。独立的 `afterEach` 在删除私有目录前释放并等待 worker,包括运行器放弃超时测试体的情况。轮询观察运行器的取消信号,teardown 独立报告事务失败,不覆盖断言失败。[PowerShell 测试](../../../../packages/shell/pwsh-local/tests/executor.spec.ts)在初始化插件前登记每个 helper 创建的 Context,并在删除临时目录前处置这些 Context。后台输入用例等待进程完成后检查完整输出、完成状态与退出码。消费式读取仍由独立的流式测试覆盖。
|
||||
|
||||
[publint runner 测试](../../../../scripts/publint-all.spec.ts)将当前测试预算传给子进程,并在解释退出码前检查启动错误和终止信号。[LSP 实例测试](../../../../packages/lsp/lsp-stdio/tests/instance.spec.ts)用同一预算等待 fixture 标记,在取消前观察实际尚未完成的 `didOpen` 写入,并在等待就绪前接住查询的 rejection。[服务器 fixture](../../../../packages/lsp/lsp-stdio/tests/fixture-server.ts)在暂停 stdin 后发布标记。Teardown 在首次 await 前捕获实例列表、Context 和目录。
|
||||
|
||||
[子 Agent 拆卸决策](2026-09-07-subagent-teardown-test-budgets.zh.md)负责生命周期清理预算。[持久 PowerShell 决策](2026-09-07-pwsh-ci-observable-completion.zh.md)负责精确与推断的终端就绪状态;一次性进程的完成 Promise 具有不同语义。
|
||||
|
||||
## 考虑过的替代方案
|
||||
|
||||
**增大独立等待时限。** 已有完成 Promise 时不采用。独立轮询期限仍会与执行通道的预算竞争。
|
||||
|
||||
**刷新反馈 golden。** 不采用:保留内容的输入框与禁用的附件按钮描述了尚未完成的提交。已稳定的预期 UI 仍是目标行为。
|
||||
|
||||
**串行化 CI 或重试这些测试。** 不采用:两者都不能建立缺失的完成条件,也不能在断言失败后释放阻塞的子进程。
|
||||
|
||||
## 后果
|
||||
|
||||
就绪与输出断言保留原有的内容和所有权检查。受控的 Desktop 就绪、Webhook 预检、命令响应、PowerShell 输出、publint 启动及 LSP 初始化延迟可复现原始失败,并在采用完成等待后通过。阻塞 Desktop worker 的控制用例仍报告测试超时,同时证明 teardown 在删除目录前等待子进程退出。执行通道分别限制测试体和清理 hook;PowerShell 和进程清理仍需在原生 Windows 上验证。
|
||||
@@ -18,6 +18,7 @@ import type { DesktopRelease } from '../src/release.ts'
|
||||
import { archivePnpmStore } from '../src/seed-store.ts'
|
||||
|
||||
const roots: string[] = []
|
||||
const releaseWorkers: Array<() => Promise<void>> = []
|
||||
|
||||
function temporaryRoot(): string {
|
||||
const root = mkdtempSync(join(tmpdir(), 'dsh-desktop-test-'))
|
||||
@@ -162,8 +163,13 @@ function release(version = '1.0.0'): DesktopRelease {
|
||||
}
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true })
|
||||
afterEach(async () => {
|
||||
const cleanups = releaseWorkers.splice(0)
|
||||
const directories = roots.splice(0)
|
||||
const results = await Promise.allSettled(cleanups.map(cleanup => cleanup()))
|
||||
for (const root of directories) rmSync(root, { recursive: true, force: true })
|
||||
const failures: unknown[] = results.flatMap((result): unknown[] => result.status === 'rejected' ? [result.reason] : [])
|
||||
if (failures.length > 0) throw new AggregateError(failures, 'desktop worker cleanup failed')
|
||||
})
|
||||
|
||||
describe('desktop package policy', () => {
|
||||
@@ -293,7 +299,7 @@ describe('desktop project transactions', () => {
|
||||
expect(existsSync(paths.pending)).toBe(false)
|
||||
})
|
||||
|
||||
it('records the live pnpm worker as transaction owner until it exits', async () => {
|
||||
it('records the live pnpm worker as transaction owner until it exits', async ({ task, signal }) => {
|
||||
const root = temporaryRoot()
|
||||
const seed = join(root, 'seed')
|
||||
const ready = join(root, 'pnpm-ready')
|
||||
@@ -306,7 +312,19 @@ describe('desktop project transactions', () => {
|
||||
const runtime = { node: process.execPath, pnpm: writeBlockingFakePnpm(root, ready, releaseWorker) }
|
||||
const manager = new DesktopProjectManager(paths, runtime)
|
||||
const installing = manager.applyRelease(seed, '1.0.0', hooks())
|
||||
await expect.poll(() => existsSync(ready)).toBe(true)
|
||||
// Teardown observes failures even if the runner has abandoned the test body.
|
||||
const completed = installing.then(value => ({ value }), (error: unknown) => ({ error }))
|
||||
releaseWorkers.push(async () => {
|
||||
writeFileSync(releaseWorker, 'continue')
|
||||
const outcome = await completed
|
||||
if ('error' in outcome) throw outcome.error
|
||||
})
|
||||
// Child startup shares the test budget; an aborted poll must not resume ownership assertions.
|
||||
await expect.poll(() => {
|
||||
signal.throwIfAborted()
|
||||
return existsSync(ready)
|
||||
}, { timeout: task.timeout }).toBe(true)
|
||||
signal.throwIfAborted()
|
||||
const workerPid = Number.parseInt(readFileSync(ready, 'utf8'), 10)
|
||||
expect(readFileSync(paths.lock, 'utf8')).toBe(`${String(workerPid)}\n`)
|
||||
const competing = new DesktopProjectManager(paths, runtime)
|
||||
|
||||
@@ -13,7 +13,7 @@
|
||||
* - LSP_FAKE_EXIT_AFTER_REPLY: "1" exits the process right after answering a textDocument/* request,
|
||||
* simulating a server that dies while idle so the pool holds a dead instance (eviction test).
|
||||
* - LSP_FAKE_OPEN_MARKER: appends each didOpen document text as one JSON line to this path.
|
||||
* - LSP_FAKE_INITIALIZED_MARKER: records when the initialized notification is received.
|
||||
* - LSP_FAKE_INITIALIZED_MARKER: records initialized receipt after any requested stdin pause.
|
||||
* - LSP_FAKE_PAUSE_STDIN_AFTER_INITIALIZED: "1" stops consuming stdin after initialized.
|
||||
* - LSP_FAKE_EXIT_DELAY_MS / LSP_FAKE_EXIT_MARKER: delay protocol exit and record exit/termination.
|
||||
* - LSP_FAKE_NO_SHUTDOWN: "1" ignores the shutdown request (forces kill escalation).
|
||||
@@ -144,8 +144,8 @@ function handle(message: { id?: number; method?: string; params?: unknown; resul
|
||||
return
|
||||
}
|
||||
if (method === 'initialized') {
|
||||
if (initializedMarker !== undefined) appendFileSync(initializedMarker, 'INITIALIZED\n')
|
||||
if (pauseStdinAfterInitialized) process.stdin.pause()
|
||||
if (initializedMarker !== undefined) appendFileSync(initializedMarker, 'INITIALIZED\n')
|
||||
return
|
||||
}
|
||||
if (method === 'textDocument/didClose') return
|
||||
|
||||
@@ -20,7 +20,7 @@ let root: string
|
||||
let ws: string
|
||||
let ctx: Context
|
||||
let fs: LocalFileSystem
|
||||
let live: LspInstance[] = []
|
||||
const live: LspInstance[] = []
|
||||
|
||||
beforeEach(async () => {
|
||||
root = await realpath(await mkdtemp(join(tmpdir(), 'lsp-inst-')))
|
||||
@@ -33,10 +33,12 @@ beforeEach(async () => {
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
for (const instance of live) await instance.dispose()
|
||||
live = []
|
||||
await ctx.fiber.dispose()
|
||||
await rm(root, { recursive: true, force: true })
|
||||
const instances = live.splice(0)
|
||||
const ownedContext = ctx
|
||||
const directory = root
|
||||
for (const instance of instances) await instance.dispose()
|
||||
await ownedContext.fiber.dispose()
|
||||
await rm(directory, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
function makeInstance(
|
||||
@@ -202,25 +204,40 @@ describe('LspInstance query and abort', () => {
|
||||
await instance.dispose()
|
||||
})
|
||||
|
||||
it('terminates when abort interrupts a backpressured didOpen write', async () => {
|
||||
it('terminates when abort interrupts a backpressured didOpen write', async ({ task, signal }) => {
|
||||
// The fixture consumes initialized, then stops reading. A document larger than the stdio pipe
|
||||
// keeps didOpen's write callback pending until cancellation forces bounded process teardown.
|
||||
await writeFile(join(ws, 'a.ts'), 'x'.repeat(2_000_000))
|
||||
const marker = join(root, 'initialized.log')
|
||||
const didOpenStarted = Promise.withResolvers<undefined>()
|
||||
let didOpenFinished = false
|
||||
const instance = makeInstance({
|
||||
LSP_FAKE_INITIALIZED_MARKER: marker,
|
||||
LSP_FAKE_PAUSE_STDIN_AFTER_INITIALIZED: '1',
|
||||
}, {
|
||||
shutdownTimeoutMs: 100,
|
||||
killGraceMs: 100,
|
||||
}, (stdin, message, done) => {
|
||||
if ((message as { method?: unknown }).method !== 'textDocument/didOpen') {
|
||||
stdin.write(encodeMessage(message), done)
|
||||
return
|
||||
}
|
||||
stdin.write(encodeMessage(message), (error) => {
|
||||
didOpenFinished = true
|
||||
done(error)
|
||||
})
|
||||
didOpenStarted.resolve(undefined)
|
||||
})
|
||||
const controller = new AbortController()
|
||||
const pending = run(instance, 'goToDefinition', controller.signal)
|
||||
await waitForFile(marker)
|
||||
// Let the client enter the large didOpen write after the fixture has paused stdin.
|
||||
await new Promise<void>(resolve => setTimeout(resolve, 100))
|
||||
const outcome = run(instance, 'goToDefinition', controller.signal)
|
||||
.then(() => undefined, (error: unknown) => error)
|
||||
await waitForFile(marker, task.timeout, signal)
|
||||
await didOpenStarted.promise
|
||||
signal.throwIfAborted()
|
||||
expect(didOpenFinished).toBe(false)
|
||||
controller.abort(new Error('didOpen-abort'))
|
||||
await expect(pending).rejects.toThrow(/didOpen-abort/)
|
||||
const failure = await outcome
|
||||
expect(() => { throw failure }).toThrow(/didOpen-abort/)
|
||||
expect(instance.dead).toBe(true)
|
||||
})
|
||||
|
||||
@@ -382,9 +399,10 @@ function failingWriter(method: string, failure = new Error(`fixture ${method} fa
|
||||
}
|
||||
|
||||
/** Wait until a fixture marker exists, bounded so a broken handshake cannot hang the test. */
|
||||
async function waitForFile(path: string, timeoutMs = 3000): Promise<void> {
|
||||
async function waitForFile(path: string, timeoutMs: number, signal: AbortSignal): Promise<void> {
|
||||
const started = Date.now()
|
||||
for (;;) {
|
||||
signal.throwIfAborted()
|
||||
try {
|
||||
await readFile(path)
|
||||
return
|
||||
|
||||
@@ -30,8 +30,14 @@ afterAll(() => {
|
||||
|
||||
/** Per-test temp dirs, removed after each test. */
|
||||
const tempDirs: string[] = []
|
||||
afterEach(() => {
|
||||
for (const dir of tempDirs.splice(0)) rmSync(dir, { recursive: true, force: true })
|
||||
const contexts: Context[] = []
|
||||
afterEach(async () => {
|
||||
const ownedContexts = contexts.splice(0)
|
||||
const directories = tempDirs.splice(0)
|
||||
const results = await Promise.allSettled(ownedContexts.map(ctx => ctx.fiber.dispose()))
|
||||
for (const dir of directories) rmSync(dir, { recursive: true, force: true })
|
||||
const failures: unknown[] = results.flatMap((result): unknown[] => result.status === 'rejected' ? [result.reason] : [])
|
||||
if (failures.length > 0) throw new AggregateError(failures, 'PowerShell fixture cleanup failed')
|
||||
})
|
||||
|
||||
// The probe follows the executor's own resolution (Program Files installs on
|
||||
@@ -51,6 +57,7 @@ function samePath(actual: string, expected: string): boolean {
|
||||
|
||||
async function setup(config: ConstructorParameters<typeof PwshLocalExecutor>[1] = {}) {
|
||||
const ctx = new Context()
|
||||
contexts.push(ctx)
|
||||
await ctx.plugin(LocalSubprocessRuntime)
|
||||
;(ctx.subprocess as LocalSubprocessRuntime).internals = { spillDir }
|
||||
// A short kill grace via the REAL config path, so escalation tests stay fast.
|
||||
@@ -422,6 +429,7 @@ describe.skipIf(!hasPwsh)('PwshLocalExecutor.start (background process handles)'
|
||||
}))
|
||||
const partialOutput = await readUntil(proc, '[bg-env][bg-dsh-env]', task.timeout)
|
||||
await proc.done
|
||||
expect(proc.status).toBe('completed')
|
||||
const output = partialOutput + lf(proc.readOutput().delta)
|
||||
expect(output).toBe('bg-stdin\n[bg-env][bg-dsh-env]\n')
|
||||
expect(proc.exitCode).toBe(0)
|
||||
|
||||
+16
-13
@@ -42,52 +42,55 @@ function fixture(options: {
|
||||
return root
|
||||
}
|
||||
|
||||
function run(root: string) {
|
||||
return spawnSync(process.execPath, [
|
||||
function run(root: string, timeoutMs: number) {
|
||||
const result = spawnSync(process.execPath, [
|
||||
'--import', 'tsx', runner,
|
||||
'--packages-root', root,
|
||||
], {
|
||||
cwd: repositoryRoot,
|
||||
encoding: 'utf8',
|
||||
timeout: 5_000,
|
||||
timeout: timeoutMs,
|
||||
})
|
||||
expect(result.error, result.stderr).toBeUndefined()
|
||||
expect(result.signal, result.stderr).toBeNull()
|
||||
return result
|
||||
}
|
||||
|
||||
describe('publint package runner', () => {
|
||||
it('lints recursively declared files from an in-memory publication view', () => {
|
||||
const result = run(fixture())
|
||||
it('lints recursively declared files from an in-memory publication view', ({ task }) => {
|
||||
const result = run(fixture(), task.timeout)
|
||||
expect(result.status, result.stderr).toBe(0)
|
||||
expect(result.stdout).toContain('linting 1 package(s)')
|
||||
expect(result.stdout).toContain('All good!')
|
||||
})
|
||||
|
||||
it('rejects an export that exists in the workspace but is not published', () => {
|
||||
const result = run(fixture({ exportPath: './unpublished.js' }))
|
||||
it('rejects an export that exists in the workspace but is not published', ({ task }) => {
|
||||
const result = run(fixture({ exportPath: './unpublished.js' }), task.timeout)
|
||||
expect(result.status).toBe(1)
|
||||
expect(result.stdout).toContain('unpublished.js')
|
||||
})
|
||||
|
||||
it('rejects a public export whose built file is missing', () => {
|
||||
const result = run(fixture({ exportPath: './lib/missing.js' }))
|
||||
it('rejects a public export whose built file is missing', ({ task }) => {
|
||||
const result = run(fixture({ exportPath: './lib/missing.js' }), task.timeout)
|
||||
expect(result.status).toBe(1)
|
||||
expect(result.stdout).toContain('missing.js')
|
||||
})
|
||||
|
||||
it('accepts published relative JavaScript and CSS targets', () => {
|
||||
it('accepts published relative JavaScript and CSS targets', ({ task }) => {
|
||||
const result = run(fixture({
|
||||
indexSource: "export { helper } from './helper.js'\nimport './theme.css'\n",
|
||||
files: {
|
||||
'lib/helper.js': 'export const helper = true\n',
|
||||
'lib/theme.css': ':root {}\n',
|
||||
},
|
||||
}))
|
||||
}), task.timeout)
|
||||
expect(result.status, result.stderr).toBe(0)
|
||||
})
|
||||
|
||||
it('rejects unpublished relative JavaScript and CSS targets', () => {
|
||||
it('rejects unpublished relative JavaScript and CSS targets', ({ task }) => {
|
||||
const result = run(fixture({
|
||||
indexSource: "export { helper } from './missing.js'\nimport './missing.css'\n",
|
||||
}))
|
||||
}), task.timeout)
|
||||
expect(result.status).toBe(1)
|
||||
expect(result.stderr).toContain('imports "./missing.js"')
|
||||
expect(result.stderr).toContain('imports "./missing.css"')
|
||||
|
||||
Reference in New Issue
Block a user