Skip to content

Commit 30360bf

Browse files
committed
fix(qa/scenarios): gallery exporter contract review — redaction, failed pages, seed tally, empty runs, shots kept
M1. With --work, the record still names the work directory of the machine that ran it (evidenceDir, every build tail and error text), and only --work was redacted. Every directory that names the run (the record's evidenceDir, --work, and the real path of each that exists here) is now replaced by $WORK in run-record.json, scorecard.json/.md, transcript.json/.md and the README, longest first and only at a path-name boundary, so /tmp never rewrites /tmpfoo. S1. A shot whose navigation answers no response or a status outside 2xx (304 counts as shown), or whose page draws one of the Console's own not-found views, is refused: skipped with the reason, its file removed, never exported as proof. S2. seedApp tallies into the half's own seed again, so an insert that throws leaves the partial inserted/refused tally in the record next to the error, as on main. S3. A runs directory with no scenario record, over an index that lists apps, throws (the CLI exits non-zero) and touches nothing. S4. An export without screenshots never replaces an app whose previous export had them: the old directory and entry are kept and the summary reports it as kept. An app that no longer passes is still removed. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ihL2ebbZkr2QAEG1x7UC8
1 parent c78ea5a commit 30360bf

8 files changed

Lines changed: 317 additions & 34 deletions

File tree

‎packages/qa/scenarios/src/export/cli.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@
1111
* file a `--driver recorded:F` run replayed); repeatable
1212
* --cards DIR the card directory (default: the repo's docs/qa/scenarios)
1313
* --work DIR read the projects here, not at each record's `evidenceDir`
14-
* --no-screenshots export without screenshots (no app is booted)
14+
* (both directories are redacted to `$WORK` in the output)
15+
* --no-screenshots export without screenshots (no app is booted); an app
16+
* whose previous export had screenshots is kept as it was
17+
*
18+
* A runs directory with no scenario record, over an index that lists apps,
19+
* exits non-zero and touches nothing (a wrong `--runs`, never "every app gone").
1520
* --chromium PATH the Chromium executable for the screenshots
1621
*
1722
* It writes only under `--out`; it pushes nothing anywhere. ⛔ It exports only
@@ -70,9 +75,10 @@ export function renderSummary(s: ExportSummary): string[] {
7075
for (const e of s.exported) lines.push(`exported ${e.id} · ${e.screenshots.length} screenshot(s)${e.skippedShots.map((k) => ` · no ${k.kind}: ${k.reason}`).join('')}`);
7176
for (const r of s.refused) lines.push(`refused ${r.id} · ${r.reasons.join(' · ')}`);
7277
for (const r of s.removed) lines.push(`removed ${r.id} · ${r.why}`);
78+
for (const k of s.kept) lines.push(`kept ${k.id} · ${k.why}`);
7379
for (const u of s.unowned) lines.push(`left apps/${u} · not in the previous index, so not the exporter's`);
7480
for (const f of s.ignoredFiles) lines.push(`ignored ${f} · not a scenario-runner record`);
75-
lines.push(`index.json ${s.indexChanged ? 'written' : 'unchanged'} · ${s.exported.length} exported, ${s.refused.length} refused, ${s.removed.length} removed`);
81+
lines.push(`index.json ${s.indexChanged ? 'written' : 'unchanged'} · ${s.exported.length} exported, ${s.refused.length} refused, ${s.removed.length} removed, ${s.kept.length} kept`);
7682
return lines;
7783
}
7884

@@ -103,7 +109,7 @@ export async function main(argv: readonly string[], deps: ExportCliDeps = {}): P
103109
...(args.work ? { workDir: resolve(args.work) } : {}),
104110
screenshots,
105111
});
106-
for (const line of renderSummary(summary)) (line.startsWith('refused') || line.startsWith('removed') ? console.error : console.log)(line);
112+
for (const line of renderSummary(summary)) (/^(refused|removed|kept) /.test(line) ? console.error : console.log)(line);
107113
return 0;
108114
}
109115

‎packages/qa/scenarios/src/export/exporter.ts‎

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
*
2121
* apps/<id>/README.md what the app is, its scorecard, the files
2222
* apps/<id>/source/… the authored metadata source (`source.ts`)
23-
* apps/<id>/run-record.json the run record, the work directory redacted
23+
* apps/<id>/run-record.json the run record, the work directories redacted (`workDirsOf`)
2424
* apps/<id>/scorecard.json|.md the per-half scorecard
2525
* apps/<id>/transcript.json|.md the per-persona 问数 transcript
2626
* apps/<id>/screenshots/<kind>.png list, kanban/calendar, record, dashboard
@@ -31,10 +31,16 @@
3131
* is gone or not generic, its project is gone — is deleted and leaves the
3232
* index; the summary names why. A directory under `apps/` the index never
3333
* listed is not the exporter's and is left alone (and named). Nothing is
34-
* deleted when the previous index does not validate.
34+
* touched when the previous index does not validate, or when the runs
35+
* directory holds no scenario record while the index lists apps (a wrong
36+
* `--runs`, never "every app gone").
37+
*
38+
* An export without screenshots (`screenshots: false`) never replaces an app
39+
* whose previous export had them: the old directory and entry are kept, and
40+
* the summary says so (`kept`). It still removes an app that no longer passes.
3541
*/
3642

37-
import { existsSync, mkdirSync, readdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs';
43+
import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, renameSync, rmSync, writeFileSync } from 'node:fs';
3844
import { dirname, join } from 'node:path';
3945

4046
import type { LoadedScenario } from '../loader.js';
@@ -95,6 +101,11 @@ export interface ExportSummary {
95101
refused: Array<{ id: string; reasons: string[] }>;
96102
/** Apps the previous index listed that this export deleted. */
97103
removed: Array<{ id: string; why: string }>;
104+
/**
105+
* Apps that would have exported, kept as the previous export left them
106+
* instead: an export without screenshots never replaces one that had them.
107+
*/
108+
kept: Array<{ id: string; why: string }>;
98109
/** Directories under `apps/` the exporter does not own, left as they were. */
99110
unowned: string[];
100111
/** JSON files in the runs directory that are not scenario-runner records. */
@@ -155,6 +166,25 @@ function writeTree(dir: string, files: Iterable<[string, string | Buffer]>): voi
155166
}
156167
}
157168

169+
/**
170+
* Every absolute directory that names this run's work directory: the record's
171+
* own `evidenceDir` (the machine that ran it — every build tail and error text
172+
* names it), `--work` when the projects are read elsewhere, and the real path
173+
* of each that exists here (a symbolic-linked temp directory). Each is redacted
174+
* from everything the gallery receives.
175+
*/
176+
export function workDirsOf(recordEvidenceDir: string, workDir: string | undefined): string[] {
177+
const dirs = [recordEvidenceDir, ...(workDir !== undefined ? [workDir] : [])];
178+
for (const d of [...dirs]) {
179+
try {
180+
dirs.push(realpathSync(d));
181+
} catch {
182+
// not on this machine: the record's directory, moved off it
183+
}
184+
}
185+
return [...new Set(dirs)];
186+
}
187+
158188
export async function exportGallery(opts: ExportOptions): Promise<ExportSummary> {
159189
const isGeneric = opts.isGeneric ?? NO_GENERIC_MARKER;
160190
const appsDir = join(opts.outDir, 'apps');
@@ -171,10 +201,17 @@ export async function exportGallery(opts: ExportOptions): Promise<ExportSummary>
171201
}
172202
}
173203
const { newest, ignored } = readRunRecords(opts.runsDir);
204+
// An empty or wrong `--runs` would otherwise read as "every app lost its record" and delete the gallery.
205+
if (newest.size === 0 && (previous?.apps.length ?? 0) > 0) {
206+
throw new Error(
207+
`${opts.runsDir} holds no scenario run record, and ${indexFile} lists ${previous!.apps.length} app(s), so nothing is touched: a runs directory with no record is read as a wrong --runs, never as every app gone`,
208+
);
209+
}
210+
const previousById = new Map((previous?.apps ?? []).map((a) => [a.id, a]));
174211
const turns = mergeRecordings(opts.recordings);
175212
const cards = new Map(opts.cards.map((s) => [s.card.id, s]));
176213

177-
const summary: ExportSummary = { exported: [], refused: [], removed: [], unowned: [], ignoredFiles: ignored, indexChanged: false };
214+
const summary: ExportSummary = { exported: [], refused: [], removed: [], kept: [], unowned: [], ignoredFiles: ignored, indexChanged: false };
178215
const entries: GalleryApp[] = [];
179216
mkdirSync(appsDir, { recursive: true });
180217

@@ -184,6 +221,14 @@ export async function exportGallery(opts: ExportOptions): Promise<ExportSummary>
184221
rmSync(staging, { recursive: true, force: true });
185222
try {
186223
const done = await exportOne(id, rf, cards.get(id), staging, isGeneric, turns, opts);
224+
const old = previousById.get(id);
225+
if (opts.screenshots === false && old && old.screenshots.length > 0 && existsSync(join(appsDir, id))) {
226+
// It still passes, but this export has no screenshots: the old directory and entry stand, shots and all.
227+
rmSync(staging, { recursive: true, force: true });
228+
entries.push(old);
229+
summary.kept.push({ id, why: `exported without screenshots, and the previous export has ${old.screenshots.length}: it is kept as it was, and not replaced by an entry with none` });
230+
continue;
231+
}
187232
rmSync(join(appsDir, id), { recursive: true, force: true });
188233
renameSync(staging, join(appsDir, id));
189234
entries.push(done.entry);
@@ -194,7 +239,7 @@ export async function exportGallery(opts: ExportOptions): Promise<ExportSummary>
194239
}
195240
}
196241

197-
const exported = new Set(entries.map((e) => e.id));
242+
const exported = new Set(entries.map((e) => e.id)); // written or kept
198243
const refusedWhy = new Map(summary.refused.map((r) => [r.id, r.reasons.join('; ')]));
199244
for (const old of previous?.apps ?? []) {
200245
if (exported.has(old.id)) continue;
@@ -237,7 +282,8 @@ async function exportOne(
237282
if (artifact === undefined) throw new Refused([`${projectDir} holds no dist/objectstack.json`]);
238283
const source = readAuthoredSource(projectDir);
239284
const frameworkVersion = projectFrameworkVersion(projectDir);
240-
const transcript = redactWorkDir(buildTranscript(scenario, record, turns), workDir);
285+
const redact = <T,>(v: T): T => redactWorkDir(v, workDirsOf(record.evidenceDir, opts.workDir));
286+
const transcript = redact(buildTranscript(scenario, record, turns));
241287

242288
let shots: PlannedShot[] = [];
243289
let skipped: SkippedShot[] = [];
@@ -247,7 +293,7 @@ async function exportOne(
247293
skipped = taken.skipped;
248294
}
249295

250-
const scorecard = redactWorkDir(record.scorecard as unknown as CardScorecard, workDir);
296+
const scorecard = redact(record.scorecard as unknown as CardScorecard);
251297
const entry: GalleryApp = {
252298
id,
253299
code: card.code,
@@ -267,12 +313,12 @@ async function exportOne(
267313

268314
writeTree(join(staging, APP_LAYOUT.source), source.files);
269315
writeTree(staging, [
270-
[APP_LAYOUT.runRecord, json(redactWorkDir(rf.raw, workDir))],
316+
[APP_LAYOUT.runRecord, json(redact(rf.raw))],
271317
[APP_LAYOUT.scorecardJson, json(scorecard)],
272318
[APP_LAYOUT.scorecardMd, `${renderScorecard(scorecard)}\n`],
273319
[APP_LAYOUT.transcriptJson, json(transcript)],
274320
[APP_LAYOUT.transcriptMd, renderTranscript(transcript, card.title)],
275-
[APP_LAYOUT.readme, appReadme(scenario, entry, judgement, scorecard, shots, skipped)],
321+
[APP_LAYOUT.readme, redact(appReadme(scenario, entry, judgement, scorecard, shots, skipped))],
276322
]);
277323
return { entry, shots, skipped };
278324
}

‎packages/qa/scenarios/src/export/record.ts‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,33 @@ export function readRunRecords(runsDir: string): ReadRecordsResult {
123123
return { newest, ignored };
124124
}
125125

126-
/** Every string in `value` with the run's local work directory replaced by `$WORK`: a local path is not gallery content. */
127-
export function redactWorkDir<T>(value: T, workDir: string): T {
128-
if (workDir.replace(/\/+$/, '') === '') return value; // `/` names no run of its own
126+
/**
127+
* A work directory is only that directory where it is not part of a longer
128+
* name: not preceded by a path-name character or `/` (`/x/tmp` is not `/tmp`),
129+
* and not followed by one (`/tmpfoo`) — a `.` that ends a sentence does not
130+
* lengthen it, a `.` that starts a suffix (`/tmp.d`) does.
131+
*/
132+
const BEFORE = '(?<![A-Za-z0-9._~/-])';
133+
const AFTER = '(?![A-Za-z0-9_~-]|\\.[A-Za-z0-9._~-])';
134+
135+
/**
136+
* Every string in `value` with each of the run's local work directories
137+
* replaced by `$WORK`: a local path is not gallery content.
138+
*
139+
* More than one directory can name the same run. The record carries the
140+
* directory of the machine that ran it (`evidenceDir`, and every build tail and
141+
* error text that names it), and `--work` names where the projects are read on
142+
* this one. Each is replaced, longest first so one that contains another is
143+
* replaced whole, and only at a path-name boundary, so `/tmp` never rewrites
144+
* `/tmpfoo`. `/` (or an empty string) names no run of its own and is skipped.
145+
*/
146+
export function redactWorkDir<T>(value: T, workDirs: string | readonly string[]): T {
147+
const dirs = [...new Set((typeof workDirs === 'string' ? [workDirs] : workDirs).map((d) => d.replace(/\/+$/, '')).filter((d) => d !== ''))].sort((x, y) => y.length - x.length);
148+
if (dirs.length === 0) return value;
149+
const escape = (d: string) => d.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
150+
const re = new RegExp(`${BEFORE}(?:${dirs.map(escape).join('|')})${AFTER}`, 'g');
129151
const walk = (v: unknown): unknown => {
130-
if (typeof v === 'string') return v.split(workDir).join('$WORK');
152+
if (typeof v === 'string') return v.replace(re, '$$WORK');
131153
if (Array.isArray(v)) return v.map(walk);
132154
if (typeof v === 'object' && v !== null) return Object.fromEntries(Object.entries(v).map(([k, x]) => [k, walk(x)]));
133155
return v;

‎packages/qa/scenarios/src/export/screenshots.ts‎

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,19 @@
2424
* - `dashboard`: a dashboard the card expects, else the build's first —
2525
* `apps/<app>/dashboard/<name>`; skipped when the build declares none.
2626
*
27+
* A page the Console did not show is never proof: a navigation that answers
28+
* no response or a status outside 2xx (304, a revalidation, is the page), or a page that draws one of the
29+
* Console's own not-found views ({@link CONSOLE_NOT_FOUND}), refuses that shot
30+
* ({@link pageRefusal}). It is reported as a skipped shot with the reason, and
31+
* no file of it is kept.
32+
*
2733
* The browser is a seam ({@link ScreenshotBrowser}): {@link playwrightBrowser}
2834
* drives Chromium through `playwright-core`; the exporter's tests pass a
2935
* double. The exporter runs this step only when asked (`--no-screenshots`
3036
* skips it), so everything else is testable without a browser.
3137
*/
3238

33-
import { existsSync, mkdirSync } from 'node:fs';
39+
import { existsSync, mkdirSync, rmSync } from 'node:fs';
3440
import { dirname, join } from 'node:path';
3541

3642
import { resolveStackCollection } from '@objectstack/core';
@@ -148,10 +154,44 @@ export function planShots(original: LoadedScenario, artifact: unknown): ShotPlan
148154

149155
// ── Capture ─────────────────────────────────────────────────────────────────
150156

157+
/**
158+
* The Console's own "nothing here" views, as a page shows them: the
159+
* `data-testid`s of the object, record and form not-found states, and the
160+
* title the app router's catch-all route draws (in the `en-US` locale the
161+
* browser runs in). Read from the Console source (`ui/packages/app-shell`,
162+
* `ui/apps/console`); a Console that drew a new one under another name would
163+
* pass this check, so the HTTP status is checked first and alone suffices for
164+
* a server error.
165+
*/
166+
export const CONSOLE_NOT_FOUND = {
167+
testIds: ['object-not-found', 'object-package-disabled', 'record-page-not-found', 'form-not-found'],
168+
titles: ['Page not found'],
169+
} as const;
170+
171+
/** What a page showed, as {@link pageRefusal} judges it. */
172+
export interface PageReading {
173+
/** The navigation's HTTP status; `null` when the navigation answered no response. */
174+
status: number | null;
175+
/** Which of {@link CONSOLE_NOT_FOUND}'s test ids and titles the page draws. */
176+
notFound: string[];
177+
}
178+
179+
/** Why a page is not a screenshot of the app, or `undefined` when it is one. */
180+
export function pageRefusal(page: PageReading): string | undefined {
181+
if (page.status === null) return 'the navigation answered no response';
182+
// 304: the browser revalidated a page it holds (each shot's page is visited twice), which is the page shown.
183+
if ((page.status < 200 || page.status > 299) && page.status !== 304) return `the navigation answered HTTP ${page.status}`;
184+
if (page.notFound.length > 0) return `the Console drew its not-found view (${page.notFound.join(', ')})`;
185+
return undefined;
186+
}
187+
188+
/** A capture's outcome: the page was shot, or it was not the app and nothing of it is proof. */
189+
export type CaptureOutcome = { shown: true } | { shown: false; reason: string };
190+
151191
/** One signed-in browser page against a booted app. */
152192
export interface ScreenshotSession {
153-
/** Open `path` under the app's origin and write a PNG to `file`. */
154-
capture(path: string, file: string): Promise<void>;
193+
/** Open `path` under the app's origin; when the page is the app ({@link pageRefusal}), write a PNG to `file`. */
194+
capture(path: string, file: string): Promise<CaptureOutcome>;
155195
close(): Promise<void>;
156196
}
157197

@@ -203,10 +243,16 @@ export async function captureScreenshots(input: CaptureInput, deps: CaptureDeps)
203243
}
204244
const session = await deps.browser.open(booted.url, deps.admin);
205245
try {
206-
for (const shot of shots) {
246+
for (const shot of [...shots]) {
207247
const file = join(input.dir, shot.file);
208248
mkdirSync(dirname(file), { recursive: true });
209-
await session.capture(shot.path, file);
249+
const outcome = await session.capture(shot.path, file);
250+
if (!outcome.shown) {
251+
rmSync(file, { force: true });
252+
shots.splice(shots.indexOf(shot), 1);
253+
skipped.push({ kind: shot.kind, reason: `the Console did not show ${shot.path}: ${outcome.reason}` });
254+
continue;
255+
}
210256
if (!existsSync(file)) throw new Error(`the browser wrote no screenshot for ${shot.kind} (${shot.path})`);
211257
}
212258
} finally {
@@ -241,11 +287,15 @@ export function playwrightBrowser(opts: PlaywrightBrowserOptions = {}): Screensh
241287
const signIn = await context.request.post(`${baseUrl}/api/v1/auth/sign-in/email`, { data: admin });
242288
if (!signIn.ok()) throw new Error(`the browser's admin sign-in was refused: ${signIn.status()}`);
243289
const page = await context.newPage();
244-
const visit = async (path: string) => {
245-
await page.goto(`${baseUrl}${path}`, { waitUntil: 'load' });
290+
const visit = async (path: string): Promise<PageReading> => {
291+
const response = await page.goto(`${baseUrl}${path}`, { waitUntil: 'load' });
246292
// A page that never goes quiet (a stream, a poll) is shot after the budget, not skipped.
247293
await page.waitForLoadState('networkidle', { timeout: opts.settleTimeoutMs ?? 30_000 }).catch(() => undefined);
248294
await page.waitForTimeout(1_000);
295+
const notFound: string[] = [];
296+
for (const id of CONSOLE_NOT_FOUND.testIds) if ((await page.locator(`[data-testid="${id}"]`).count()) > 0) notFound.push(id);
297+
for (const title of CONSOLE_NOT_FOUND.titles) if ((await page.getByText(title, { exact: true }).count()) > 0) notFound.push(`"${title}"`);
298+
return { status: response ? response.status() : null, notFound };
249299
};
250300
// An administrator's first visit to an app page can open first-run UI over it (measured on
251301
// 18.3: a default-currency dialog over the first app page, once per browser session; the
@@ -258,8 +308,10 @@ export function playwrightBrowser(opts: PlaywrightBrowserOptions = {}): Screensh
258308
await visit(path);
259309
warmed = true;
260310
}
261-
await visit(path);
311+
const refusal = pageRefusal(await visit(path));
312+
if (refusal !== undefined) return { shown: false, reason: refusal };
262313
await page.screenshot({ path: file });
314+
return { shown: true };
263315
},
264316
close: () => browser.close(),
265317
};

0 commit comments

Comments
 (0)