Skip to content

Commit 63d1992

Browse files
committed
fix(cli): the #12525 read-back's anti-vacuity pins read CODE, and bind the argument rather than the argument list
`serve-port-readback.e2e.test.ts` runs only in the nightly `e2e` tier, so no pull request can redden it. Two source-text pins in its ANTI-VACUITY describe read the raw file, and both went red on `main` for reasons that were never about the product: - the print-order pin did `indexOf` over the whole of `src/utils/format.ts`. #17892 added a docblock quoting `Press Ctrl+C to stop` 281 lines above the `console.error` that prints it, so the raw read put the tail at offset 38075 and the `API:` row at 47343 and the assertion reported a print order that had never changed. In code position the two are 47343 and 53551, i.e. line 984 before line 1081, exactly as the pin claims. - the banner pin held the byte-exact `resolveAuthBaseUrl(boundPort)` call. #17725 added a `boundProtocol` argument. The per-PR sibling `src/commands/serve-bound-port-publication.test.ts` was updated in that same commit; this nightly-only copy could not be, and had no way to say so until the next sweep. Both pins now read through `scripts/js-comment-mask.mjs`, the tree's one answer to code-versus-prose, which also closes the other direction: a comment naming a pinned spelling can no longer satisfy the pin with no code behind it. The mask carries its own anti-vacuity control (blanked in place, and blanked something). The banner pin now binds the ARGUMENT — the row is derived from `boundPort` — and tolerates whatever else the call grows, which is the division of labour the two tiers imply: the per-PR sibling keeps the byte-exact line and reddens on the PR that moves it. That this is narrower rather than looser is proven in the test on synthetic text: the matcher rejects `resolveAuthBaseUrl(port)` and `resolveAuthBaseUrl(requestedPort, boundProtocol)`, and a standing negative asserts neither requested-port spelling is in `serve.ts`. ⛔ No test renamed, skipped or deleted, and no assertion dropped: the describe gains four assertions and loses none. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8cf527f commit 63d1992

1 file changed

Lines changed: 87 additions & 4 deletions

File tree

‎packages/cli/test/serve-port-readback.e2e.test.ts‎

Lines changed: 87 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ import { tmpdir } from 'node:os';
5252
import { join, resolve } from 'node:path';
5353
import { fileURLToPath } from 'node:url';
5454

55+
// The tree's ONE answer to "is this span code, or prose" — the same import
56+
// `test/vitest-tiers-partition.test.ts` and `src/commands/
57+
// serve-bound-port-publication.test.ts` use. The ANTI-VACUITY describe below
58+
// is the only consumer here; see its header for why a raw read is unsound in
59+
// BOTH directions.
60+
import { maskComments } from '../../../scripts/js-comment-mask.mjs';
61+
5562
import {
5663
boundPortFromBanner,
5764
holdPort,
@@ -198,10 +205,45 @@ describe('#12525: the child\'s REAL port is read back out of its own banner', ()
198205
// above stays green while `boundPortFromBanner()` returns `no-banner`
199206
// forever — which is a SILENT SKIP on every real boot, i.e. the exact false
200207
// green this card is about, restored with nothing red to show for it.
201-
const formatSource = () => readFileSync(resolve(HERE, '../src/utils/format.ts'), 'utf8');
208+
//
209+
// ## ⛔ EVERY read in this describe is of CODE, never of the raw file
210+
//
211+
// `scripts/js-comment-mask.mjs` blanks comment spans in place — spaces for
212+
// comment bytes, newlines kept — so every offset and line number survives
213+
// the mask and an `indexOf` still points at the line it names.
214+
//
215+
// A raw read is unsound in BOTH directions, and this file has now been bitten
216+
// by each of them on a nightly that no pull request could have reddened:
217+
//
218+
// - it FABRICATES. `format.ts` gained a docblock quoting
219+
// `Press Ctrl+C to stop` 281 lines ABOVE the `console.error` that prints
220+
// it, so a whole-file `indexOf` put the tail before the `API:` row and
221+
// the order assertion reported a print order that had never changed.
222+
// - it VANISHES a pin. A comment naming the exact spelling a `toContain`
223+
// looks for satisfies that pin with no code behind it — the vacuum this
224+
// whole describe exists to prevent, reintroduced by its own instrument.
225+
//
226+
// Reading the raw bytes too is deliberate: the mask's own anti-vacuity
227+
// control needs something to compare against.
228+
const sourceOf = (relPath: string): { raw: string; code: string } => {
229+
const raw = readFileSync(resolve(HERE, relPath), 'utf8');
230+
return { raw, code: maskComments(raw) };
231+
};
232+
233+
/**
234+
* The mask ran, blanked in place, and blanked SOMETHING. Without it every
235+
* negative below would pass against an empty string and every positive
236+
* would fail for a reason that is not about the source under test.
237+
*/
238+
const expectMasked = ({ raw, code }: { raw: string; code: string }): void => {
239+
expect(code.length, 'the mask changed the file length — offsets no longer line up').toBe(raw.length);
240+
expect(code, 'the mask returned the file unchanged — it blanked no comment at all').not.toBe(raw);
241+
};
202242

203243
it('printServerReady still prints an `API:` row and still ends with the tail', () => {
204-
const source = formatSource();
244+
const format = sourceOf('../src/utils/format.ts');
245+
expectMasked(format);
246+
const source = format.code;
205247
const apiAt = source.indexOf('API:');
206248
const tailAt = source.indexOf('Press Ctrl+C to stop');
207249

@@ -238,13 +280,54 @@ describe('#12525: the child\'s REAL port is read back out of its own banner', ()
238280
// number that was REQUESTED and it stays 0 under `--port 0` (#13062).
239281
// The banner now reads the transport's own answer, so the premise below
240282
// is the stronger one it was always meant to be.
241-
const serveSource = readFileSync(resolve(HERE, '../src/commands/serve.ts'), 'utf8');
283+
//
284+
// ## ⭐ The ARGUMENT is the invariant; the argument LIST is not
285+
//
286+
// This same claim is pinned twice in this package, on purpose and at two
287+
// different strengths, because the two copies run in different tiers:
288+
//
289+
// - `src/commands/serve-bound-port-publication.test.ts` runs per-PR
290+
// (`queue`) and holds the BYTE-EXACT call, `boundProtocol` argument
291+
// and all. It reddens on the pull request that rewords the line, with
292+
// that PR's author reading the failure.
293+
// - this file runs ONLY on the nightly `main` sweep, so an exact-spelling
294+
// pin here cannot be kept honest by the PR that moves the spelling: it
295+
// goes red a day later, on a card nobody can attribute. That is not a
296+
// hypothetical — it is what happened, and what this test is being
297+
// repaired from. So this copy binds the SEMANTIC premise the read-back
298+
// rests on and nothing more: whatever else the call grows, the port it
299+
// is handed is the BOUND one.
300+
//
301+
// ⛔ Binding less is not binding loosely — the matcher still fails on every
302+
// spelling that would make this file vacuous, and that discrimination is
303+
// PROVEN below on synthetic text rather than asserted, so the proof holds
304+
// whatever `serve.ts` goes on to say.
305+
const BANNER_FROM_BOUND_PORT = /externalBaseOrigin:\s*resolveAuthBaseUrl\(\s*boundPort\s*[,)]/;
306+
const BANNER_FROM_REQUESTED_PORT = /externalBaseOrigin:\s*resolveAuthBaseUrl\(\s*(?:port|requestedPort)\s*[,)]/;
307+
308+
expect('externalBaseOrigin: resolveAuthBaseUrl(boundPort).baseOrigin').toMatch(BANNER_FROM_BOUND_PORT);
309+
expect('externalBaseOrigin: resolveAuthBaseUrl(boundPort, boundProtocol).baseOrigin')
310+
.toMatch(BANNER_FROM_BOUND_PORT);
311+
// ⛔ …and the regression it exists to catch does NOT satisfy it, in either
312+
// of the two names the requested port goes by in `run()`.
313+
expect('externalBaseOrigin: resolveAuthBaseUrl(port).baseOrigin').not.toMatch(BANNER_FROM_BOUND_PORT);
314+
expect('externalBaseOrigin: resolveAuthBaseUrl(requestedPort, boundProtocol).baseOrigin')
315+
.not.toMatch(BANNER_FROM_BOUND_PORT);
316+
317+
const serve = sourceOf('../src/commands/serve.ts');
318+
expectMasked(serve);
319+
const serveSource = serve.code;
242320
expect(serveSource).toContain('port = await getAvailablePort(requestedPort)');
243321
expect(
244322
serveSource,
245323
'the ready banner no longer derives its API row from `resolveAuthBaseUrl(boundPort)` — '
246324
+ 'if it now uses the REQUESTED port, the #12525 read-back is vacuous by construction',
247-
).toContain('externalBaseOrigin: resolveAuthBaseUrl(boundPort).baseOrigin');
325+
).toMatch(BANNER_FROM_BOUND_PORT);
326+
expect(
327+
serveSource,
328+
'the banner row is built from the REQUESTED port — #12525 read-back is vacuous: '
329+
+ 'the harness can never disagree with a banner derived from what it asked for',
330+
).not.toMatch(BANNER_FROM_REQUESTED_PORT);
248331
expect(
249332
serveSource,
250333
'the bound port is no longer resolved off the transport — `boundPort` is what the '

0 commit comments

Comments
 (0)