Skip to content

Commit d7d111f

Browse files
test(cli): 把 NDJSON e2e 的放行截止时间锚定到 device code 签发时刻 (#6855) (#6873)
`cloud-login-json-ndjson.e2e.test.ts:333` 的 `releasedByDeadline` 断言在合并 队列里间歇性变红,已经先后把两个与之无关的 PR 踢出队列(#6847 仅改 spec、 #6835 仅改 docs)。 根因不是契约被破坏,而是逃生阀计时器的**锚点**错了:它在 `execFile` 之前 就已武装,于是 `script(1)` 启动、`tsx` 对整棵 oclif 命令树的转译、模块加载 全都被计入这份 20s 预算 —— 而这份预算存在的目的只是监督"设备记录必须尽早 落到 stdout"。空载实测: | 区间 | 实测 | |---|---| | spawn → 请求 device code(纯启动) | 3423–3713 ms | | device code 响应 → 记录可读(真正的契约窗口) | 16–28 ms | 即旧预算约 99.4% 花在契约管不着的启动上,启动只需慢 5.7 倍即可耗尽 20s —— 对一个并发 84 个 task、跑满 11 分半的队列 runner 来说完全是常态。 改法是把计时器改为在端点签发 device code 的那一刻才武装:那是契约第一次 可被观测的时刻,此后 CLI 手上已经握有 verification URL。`RELEASE_DEADLINE_MS` 数值保持 20s 不变 —— 这是**重新锚定**,不是放宽超时;重新锚定后被守护的窗口 从 ~3500ms 启动变成 ~20ms 契约窗口,余量约 1000 倍。 断言本身没有被削弱:反向验证把 `onDeviceCode` 限制退回 #6730 的缓冲式实现后, 该断言依旧变红(5 failed | 7 passed),因此 #6531/#6730 的"URL 先于授权"保证 仍然被咬住。顺带把断言消息改准确 —— 旧文案"never reached stdout early"在启动 超时的情形下是错误诊断。 Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5a214ac commit d7d111f

1 file changed

Lines changed: 55 additions & 7 deletions

File tree

packages/cli/test/cloud-login-json-ndjson.e2e.test.ts

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,30 @@ const DEPLOY_DOCS = resolve(REPO_ROOT, 'content/docs/deployment/index.mdx');
8484
* anyway. Only reached when the record never arrives early — i.e. when the
8585
* contract is broken — and exists so that failure is an assertion rather than a
8686
* suite that hangs until the runner kills it.
87+
*
88+
* ## The clock starts at device-code issuance, not at spawn (#6855)
89+
*
90+
* This budget is armed when the endpoint hands the CLI its device code, because
91+
* that is the first instant at which the contract is even measurable: from
92+
* there the CLI holds the verification URL and owes it to stdout. Everything
93+
* before it — `script(1)`, the `tsx` transform of the whole oclif command tree,
94+
* module loading — is process startup, about which #6531/#6730 say nothing.
95+
*
96+
* Armed at spawn instead, this budget policed startup rather than the contract,
97+
* and that is what made the assertion flaky. Measured on an idle machine, of
98+
* the latency from spawn to the record being readable:
99+
*
100+
* | segment | measured |
101+
* |--------------------------------------|-----------|
102+
* | spawn → device-code request (startup)| 3423–3713 ms |
103+
* | device-code response → record readable (the contract) | 16–28 ms |
104+
*
105+
* So ~99.4% of the old budget was spent on work the contract does not govern,
106+
* leaving startup needing only a ~5.7x slowdown to exhaust 20 s — routine on a
107+
* merge-queue runner executing 84 tasks for 11½ minutes. It duly ejected two
108+
* unrelated PRs (#6847 spec-only, #6835 docs-only). Anchored here, the budget
109+
* covers a ~20 ms window with ~1000x headroom, and the number itself is
110+
* unchanged: this is a re-anchoring, NOT a widened timeout.
87111
*/
88112
const RELEASE_DEADLINE_MS = 20_000;
89113

@@ -111,6 +135,14 @@ function startDeviceEndpoint(outcome: 'token' | 'access_denied') {
111135
let released = false;
112136
let authorizedAt: number | null = null;
113137

138+
// Resolved the moment the CLI has been handed its device code — the instant
139+
// the emission contract starts running, and so the anchor for the release
140+
// deadline. Definite-assignment: the Promise executor runs synchronously.
141+
let markDeviceCodeIssued!: () => void;
142+
const deviceCodeIssued = new Promise<void>((res) => {
143+
markDeviceCodeIssued = res;
144+
});
145+
114146
const server: Server = createServer((req, res) => {
115147
req.resume();
116148
req.on('end', () => {
@@ -121,6 +153,7 @@ function startDeviceEndpoint(outcome: 'token' | 'access_denied') {
121153
const { pathname } = new URL(req.url ?? '/', 'http://placeholder');
122154

123155
if (pathname === '/api/v1/auth/device/code') {
156+
markDeviceCodeIssued();
124157
return send(200, {
125158
device_code: 'DEV-CODE-6730',
126159
user_code: 'WXYZ-6730',
@@ -153,6 +186,7 @@ function startDeviceEndpoint(outcome: 'token' | 'access_denied') {
153186
release: () => {
154187
released = true;
155188
},
189+
deviceCodeIssued,
156190
authorizedAt: () => authorizedAt,
157191
listen: () =>
158192
new Promise<number>((res) => {
@@ -218,12 +252,20 @@ async function runCloudDeviceLogin(opts: {
218252
}
219253
}, WATCH_MS);
220254

221-
const deadline = setTimeout(() => {
222-
if (urlSeenAt === null) {
223-
releasedByDeadline = true;
224-
endpoint.release();
225-
}
226-
}, RELEASE_DEADLINE_MS);
255+
// Armed on device-code issuance rather than here, so the budget covers the
256+
// window the contract governs and not the child's startup — see
257+
// {@link RELEASE_DEADLINE_MS} for the measurements behind that (#6855).
258+
// Stays unarmed if the CLI never reaches the device flow at all; that run
259+
// ends when the child exits, and the assertions below name the absence.
260+
let deadline: ReturnType<typeof setTimeout> | undefined;
261+
void endpoint.deviceCodeIssued.then(() => {
262+
deadline = setTimeout(() => {
263+
if (urlSeenAt === null) {
264+
releasedByDeadline = true;
265+
endpoint.release();
266+
}
267+
}, RELEASE_DEADLINE_MS);
268+
});
227269

228270
const shell = [
229271
`'${TSX}' '${CLI}' cloud login${opts.json ? ' --json' : ''} --no-browser`,
@@ -330,7 +372,13 @@ describe('os cloud login --json — the declared NDJSON stream (#6730)', () => {
330372
it('hands over the verification URL BEFORE authorization — the reason this route was chosen', () => {
331373
// The endpoint only authorized because the watcher had already read the
332374
// record off stdout, so these two facts are the run's own history.
333-
expect(ok.releasedByDeadline, 'the device record never reached stdout early').toBe(false);
375+
expect(
376+
ok.releasedByDeadline,
377+
`the device record did not reach stdout within ${RELEASE_DEADLINE_MS}ms of the CLI ` +
378+
'receiving its device code — the buffered-emit regression this route was chosen to ' +
379+
'prevent. This window excludes process startup (#6855), so a slow runner is not a ' +
380+
'cause: at the moment it opens the CLI already holds the verification URL.',
381+
).toBe(false);
334382
expect(ok.urlSeenAt).not.toBeNull();
335383
expect(ok.authorizedAt).not.toBeNull();
336384
expect(ok.urlSeenAt!).toBeLessThanOrEqual(ok.authorizedAt!);

0 commit comments

Comments
 (0)