Repository navigation
Add the real-model browser E2E suite (#204) - #225
Conversation
5c1b388 to
c02417d
Compare
hsliuustc0106
left a comment
There was a problem hiding this comment.
Needs changes before merge (cannot request-changes on own PR from this account).
Evidence upload is not fail-closed on a public, secret-bearing workflow. Two blocking notes inline; suite design otherwise looks solid.
| # Only the sanitizer-cleared allow-list is uploaded: the report, the screenshots, | ||
| # the transcript — never the bench home, the tap log or any raw log. | ||
| - name: Upload evidence (allow-list, sanitizer-checked) | ||
| if: always() |
There was a problem hiding this comment.
Blocking: if: always() uploads evidence/ even when run.mjs marked the bundle unfit (sanitizerFindings). That contradicts the fail-closed claim in the workflow header and in sanitize.mjs. Gate this step on a clean scan (or wipe/omit contaminated paths) so findings never become a public artifact.
There was a problem hiding this comment.
Fixed at aaf69f1: the runner writes evidence/sanitized.txt only after a clean scan (and after report.json is final), and the upload step requires that stamp through an explicit evidence-gate step — a flagged bundle, or a run that never reached the scan, uploads nothing.
| const findings = [] | ||
| for (const [name, pattern] of [['login token URL', TOKEN_URL], ['cookie material', COOKIE]]) { | ||
| const hit = text.match(pattern) | ||
| if (hit) findings.push(`${name}: ${hit[0].slice(0, 48)}…`) |
There was a problem hiding this comment.
Blocking: findings embed hit[0].slice(0, 48), so a ?token= match can land in report.sanitizerFindings and then in uploaded report.json. Prefer a redacted label (e.g. login token URL) with no secret material.
There was a problem hiding this comment.
Fixed at aaf69f1: findings are now kind-only labels (login token URL / cookie material / the API key value appears verbatim) plus the file path — the matched text never enters a finding, a report or an artifact. Locked by a test that asserts the token/key bytes do not appear in the findings string.
Four app-independent scenario checks (fixture CSV in, summary.json / summary-east.json out, reload survival, sidebar previews) with one FlyBot web adapter, a real-model bench launcher and a fail-closed runner (guards, budgets, outcome contract, evidence sanitizer), run nightly on main by a dedicated secret-bearing workflow. Scenario logic is covered keyless in verify through a fake adapter, following the dev/interop pattern; Orbit reuses the scenarios through its own adapter (#209).
c02417d to
aaf69f1
Compare
Fixes #204. Follow-up: #209 (Orbit adapter, not exercised). Flake noticed on the way: #224.
The four #204 checks as app-independent scenario code (fixture CSV →
summary.json/summary-east.json→ reload survival → sidebar previews, asserted by parsed JSON and SHA-256, never by a filename echoed in a reply), one FlyBot web adapter (playwright-core from the harness checkout, UI task steps, read-only observation), a real-model bench launcher (dev/live-e2e/bench.mjs) and a fail-closed runner (guards, budgets,pass|assertion_failed|provider_failed|infra_failed|budget_exceeded→ exit 0–4, structured report, evidence sanitizer). Nightly at 00:30 Asia/Shanghai + dispatch, main-only, window-gated, secret step-scoped — the repo's first secret-bearing workflow. Scenario logic is covered keyless innpm run verifyvia a fake adapter (dev/interop pattern); the live command never runs in verify. Decisions per the readiness review are recorded in the #204 comment; both blocking review findings (upload gated on a clean scan, findings redacted to kind-only labels) are fixed.Verifier. Tip
aaf69f1(CItests-okgreen at this SHA — oneplugins (social)run hit the #224 flake and passed on rerun):npm run verify(all suites green; includes the newlint:live-e2e/test:live-e2e, 16 tests), fail-closed checks ofbench.mjs/run.mjs(missing key, mock key,~/.dshhome, loopback without tap upstream — all refuse with explicit errors, nothing written), a full local bench cycle (boot with a shape-valid dummy key — plugin active,bot/state.jsoncreated; stop frees the port), simulated clean/contaminated evidence scans (kind-only findings, no matched text, stamp written only when clean).DEEPSEEK_API_KEYwas provided to this session — the maintainer adds the secret before the first dispatch), the workflow on CI (needsmain; dispatch it after merge for the first real run), and the 5-consecutive-runs stability claim (Add real-model browser E2E nightly tests with reusable scenarios #204 requires them before "stable" is claimed).