diff --git a/packages/cli/test/login-json-ndjson.e2e.test.ts b/packages/cli/test/login-json-ndjson.e2e.test.ts index c232a7ab62..9538e77eb9 100644 --- a/packages/cli/test/login-json-ndjson.e2e.test.ts +++ b/packages/cli/test/login-json-ndjson.e2e.test.ts @@ -80,6 +80,35 @@ const AUTH_DOCS = resolve(REPO_ROOT, 'content/docs/permissions/authentication.md * anyway. Only reached when the record never arrives early — i.e. when the * contract is broken — and exists so that failure is an assertion rather than a * suite that hangs until the runner kills it. + * + * ## The clock starts at device-code issuance, not at spawn (#6872, shape from #6855) + * + * This budget is armed when the endpoint hands the CLI its device code, because + * that is the first instant at which the contract is even measurable: from + * there the CLI holds the verification URL and owes it to stdout. Everything + * before it — `script(1)`, the `tsx` transform of the whole oclif command tree, + * module loading — is process startup, about which #6531 says nothing. + * + * Armed at spawn instead, this budget policed startup rather than the contract. + * Measured on this file (10 runs, probe replicating {@link runDeviceLogin}), of + * the latency from spawn to the record being readable: + * + * | segment | measured | + * |-------------------------------------------------------|--------------| + * | spawn → device-code request (startup) | 6844–9587 ms | + * | device-code response → record readable (the contract) | 13–39 ms | + * + * So ~99.7% of the old budget was spent on work the contract does not govern, + * leaving startup needing only a ~2.1x slowdown to exhaust 20 s. The spread in + * that row is itself the argument: the same probe measured 6844 ms on a quiet + * box and 9587 ms once other worktrees started building, with nothing about + * the contract having changed in between. The cloud sibling, on this identical + * harness, duly ejected two unrelated PRs from the merge queue (#6847 + * spec-only, #6835 docs-only) before #6855 re-anchored it. + * + * Anchored here, the budget covers a ~20 ms window with ~500x headroom, and + * the number itself is unchanged: this is a re-anchoring, NOT a widened + * timeout. */ const RELEASE_DEADLINE_MS = 20_000; @@ -107,6 +136,14 @@ function startDeviceEndpoint(outcome: 'token' | 'access_denied') { let released = false; let authorizedAt: number | null = null; + // Resolved the moment the CLI has been handed its device code — the instant + // the emission contract starts running, and so the anchor for the release + // deadline. Definite-assignment: the Promise executor runs synchronously. + let markDeviceCodeIssued!: () => void; + const deviceCodeIssued = new Promise((res) => { + markDeviceCodeIssued = res; + }); + const server: Server = createServer((req, res) => { req.resume(); req.on('end', () => { @@ -117,6 +154,7 @@ function startDeviceEndpoint(outcome: 'token' | 'access_denied') { const { pathname } = new URL(req.url ?? '/', 'http://placeholder'); if (pathname === '/api/v1/auth/device/code') { + markDeviceCodeIssued(); return send(200, { device_code: 'DEV-CODE-6531', user_code: 'WXYZ-6531', @@ -147,6 +185,7 @@ function startDeviceEndpoint(outcome: 'token' | 'access_denied') { return { server, release: () => { released = true; }, + deviceCodeIssued, authorizedAt: () => authorizedAt, listen: () => new Promise((res) => { @@ -202,12 +241,20 @@ async function runDeviceLogin(outcome: 'token' | 'access_denied'): Promise { - if (urlSeenAt === null) { - releasedByDeadline = true; - endpoint.release(); - } - }, RELEASE_DEADLINE_MS); + // Armed on device-code issuance rather than here, so the budget covers the + // window the contract governs and not the child's startup — see + // {@link RELEASE_DEADLINE_MS} for the measurements behind that (#6872). + // Stays unarmed if the CLI never reaches the device flow at all; that run + // ends when the child exits, and the assertions below name the absence. + let deadline: ReturnType | undefined; + void endpoint.deviceCodeIssued.then(() => { + deadline = setTimeout(() => { + if (urlSeenAt === null) { + releasedByDeadline = true; + endpoint.release(); + } + }, RELEASE_DEADLINE_MS); + }); const shell = [ `'${TSX}' '${CLI}' login --json --no-browser`, @@ -289,7 +336,13 @@ describe('os login --json — the declared NDJSON stream (#6531)', () => { it('hands over the verification URL BEFORE authorization — the reason this route was chosen', () => { // The endpoint only authorized because the watcher had already read the // record off stdout, so these two facts are the run's own history. - expect(ok.releasedByDeadline, 'the device record never reached stdout early').toBe(false); + expect( + ok.releasedByDeadline, + `the device record did not reach stdout within ${RELEASE_DEADLINE_MS}ms of the CLI ` + + 'receiving its device code — the buffered-emit regression this route was chosen to ' + + 'prevent. This window excludes process startup (#6872), so a slow runner is not a ' + + 'cause: at the moment it opens the CLI already holds the verification URL.', + ).toBe(false); expect(ok.urlSeenAt).not.toBeNull(); expect(ok.authorizedAt).not.toBeNull(); expect(ok.urlSeenAt!).toBeLessThanOrEqual(ok.authorizedAt!);