fix(live-e2e): read DEEPSEEK_API_KEY once for the preflight, the bench and the guard

.github/scripts/live-e2e-preflight.mjs

@@ -1,14 +1,19 @@
 #!/usr/bin/env node
 // The live E2E preflight (#794). The suite cannot run without DEEPSEEK_API_KEY, and a run that is
-// red every night for a reason nobody can act on teaches the team to ignore red. So a missing key
-// is a warning, not a failure: this step publishes `key=false`, the workflow skips every step that
-// needs the key, and the summary says the run measured nothing. A failure inside the suite still
-// fails the run, so the skip is never a pass.
+// red every night for a reason nobody can act on teaches the team to ignore red. So a key the suite
+// cannot use is a warning, not a failure: this step publishes `key=false`, the workflow skips every
+// step that needs the key, and the summary says the run measured nothing. A failure inside the
+// suite still fails the run, so the skip is never a pass.
+//
+// "Cannot use" is the suite's own reading (`dev/live-e2e/key.mjs`), not this step's: the bench and
+// the run guard refuse a key under 16 characters or the mock's, and a preflight that accepted them
+// let those keys through to a red bench instead of the skip.
 //
 // `.github/workflows/live-e2e.yml` runs it as `node .github/scripts/live-e2e-preflight.mjs`, with
 // the secret mapped into DEEPSEEK_API_KEY step-scoped and nothing else in the environment.
 import { appendFileSync } from 'node:fs'
 import { fileURLToPath } from 'node:url'
+import { keyReading } from '../../dev/live-e2e/key.mjs'
 
 export const WARNING = '::warning title=Live E2E skipped::DEEPSEEK_API_KEY is not configured as a repository secret; the four real-model checks and the evidence bundle are skipped, so this run measured nothing'
 
@@ -20,15 +25,26 @@ export const SUMMARY = [
   '**This run is green because there was nothing to run, not because the suite passed.** Add the secret in Settings → Secrets and variables → Actions to measure the suite again (#794).',
 ]
 
+export const UNUSABLE_WARNING = '::warning title=Live E2E skipped::DEEPSEEK_API_KEY is set but is not a usable key (the mock\'s key, or shorter than 16 characters); the four real-model checks and the evidence bundle are skipped, so this run measured nothing'
+
+export const UNUSABLE_SUMMARY = [
+  '### Live E2E skipped: the repository secret is not a usable key',
+  '',
+  '`DEEPSEEK_API_KEY` is set, but the suite refuses it: the mock\'s key or fewer than 16 characters never reaches the real endpoint, so no step of the suite ran and no evidence bundle was produced.',
+  '',
+  '**This run is green because there was nothing to run, not because the suite passed.** Replace the secret in Settings → Secrets and variables → Actions with a real key to measure the suite again.',
+]
+
 // A secret that is not configured reaches the step as an empty string, not as a missing variable,
 // so both have to mean the same thing here.
 export function plan(key) {
-  const present = typeof key === 'string' && key.length > 0
+  const { usable, reason } = keyReading(key)
+  const missing = reason === 'missing'
   return {
-    present,
-    output: `key=${present}`,
-    warning: present ? null : WARNING,
-    summary: present ? null : SUMMARY.join('\n'),
+    present: usable,
+    output: `key=${usable}`,
+    warning: usable ? null : (missing ? WARNING : UNUSABLE_WARNING),
+    summary: usable ? null : (missing ? SUMMARY : UNUSABLE_SUMMARY).join('\n'),
   }
 }
 

.github/scripts/live-e2e-preflight.test.mjs

@@ -11,7 +11,7 @@ import { join } from 'node:path'
 import { SUMMARY, WARNING, plan } from './live-e2e-preflight.mjs'
 
 test('#794: a configured key runs the suite and says nothing', () => {
-  const step = plan('sk-live-e2e')
+  const step = plan('sk-live-e2e-0123456789abcdef')
   assert.equal(step.output, 'key=true')
   assert.equal(step.warning, null)
   assert.equal(step.summary, null)
@@ -52,10 +52,23 @@ test('#794: with a key the step writes key=true and adds no warning or summary',
   writeFileSync(summary, '')
   const result = spawnSync(process.execPath, [join(import.meta.dirname, 'live-e2e-preflight.mjs')], {
     encoding: 'utf8',
-    env: { PATH: process.env.PATH, DEEPSEEK_API_KEY: 'sk-live-e2e', GITHUB_OUTPUT: output, GITHUB_STEP_SUMMARY: summary },
+    env: { PATH: process.env.PATH, DEEPSEEK_API_KEY: 'sk-live-e2e-0123456789abcdef', GITHUB_OUTPUT: output, GITHUB_STEP_SUMMARY: summary },
   })
   assert.equal(result.status, 0)
   assert.equal(result.stdout, '')
   assert.equal(readFileSync(output, 'utf8').trim(), 'key=true')
   assert.equal(readFileSync(summary, 'utf8'), '')
 })
+
+test('a key that is present but unusable skips like a missing one, and says why', () => {
+  assert.equal(plan('   ').warning, WARNING, 'a blank secret is the same thing as no secret')
+  for (const key of ['mock-key', 'short', 'a'.repeat(15)]) {
+    const step = plan(key)
+    assert.equal(step.output, 'key=false', `the preflight lets ${JSON.stringify(key)} through`)
+    assert.match(step.warning, /^::warning /)
+    assert.match(step.warning, /not a usable key/)
+    assert.notEqual(step.warning, WARNING, 'a key someone set is a different warning from no key at all')
+    assert.match(step.summary, /not a usable key/)
+    assert.match(step.summary, /nothing to run, not because the suite passed/)
+  }
+})

changelog.d/2214-live-e2e-key-reading.changed.md

@@ -0,0 +1 @@
+A `Live E2E` run no longer publishes a key the suite itself refuses: the preflight that decides whether any step runs, the bench starter and the run guard now read `DEEPSEEK_API_KEY` through one module, so the mock's key or one under 16 characters is skipped with its own warning — saying plainly that nothing ran — instead of reaching a bench that refuses it and failing the night for a reason the skip exists to avoid. A key long enough but not real is not detected here; only the endpoint can judge that ([#2214](https://github.com/ThinkFlowLab/FlyBot/issues/2214)).

dev/live-e2e/README.md

@@ -57,7 +57,7 @@ a real `DEEPSEEK_API_KEY` in the environment. Chrome comes from `CHROME_BIN`, el
 `google-chrome`/`chromium`(-`browser`) on PATH, else playwright's own download.
 
 ```sh
-export DEEPSEEK_API_KEY=...            # real key; the runner fails closed without it
+export DEEPSEEK_API_KEY=...            # real key (16+ chars, never the mock's); fails closed
 export LIVE_E2E_HOME="$HOME/.cache/flybot-live-e2e/live-e2e"   # outside the checkout
 node dev/live-e2e/bench.mjs live-e2e 3196 reset
 node dev/live-e2e/run.mjs "$LIVE_E2E_HOME" 3196 --model deepseek-official/deepseek-flash

dev/live-e2e/bench.mjs

@@ -8,14 +8,16 @@
 // Data lives outside this checkout — `$LIVE_E2E_HOME`, else ~/.cache/flybot-live-e2e/<name> (never
 // under ~/.dsh; `reset` wipes the Bot team and Sessions before starting). A home inside the
 // checkout is refused: that workspace would hold the grader (#398). Environment: DEEPSEEK_API_KEY
-// (required; fails loudly when missing or the mock's), DEEPSEEK_BASE_URL (optional; defaults to the
-// official endpoint — point it at dev/llm-tap.mjs to record per-call usage), DSH_HARNESS (a
-// deepseek-harness checkout with its dependencies installed).
+// (required; fails loudly when the suite's own reading refuses it — see key.mjs),
+// DEEPSEEK_BASE_URL (optional; defaults to the official endpoint — point it at
+// dev/llm-tap.mjs to record per-call usage), DSH_HARNESS (a deepseek-harness checkout with
+// its dependencies installed).
 import { spawn, spawnSync } from 'node:child_process'
 import { existsSync, mkdirSync, openSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
 import { dirname, join } from 'node:path'
 import { fileURLToPath } from 'node:url'
 import { benchHome } from './integrity.mjs'
+import { keyProblem, keyReading } from './key.mjs'
 
 const ROOT = dirname(dirname(dirname(fileURLToPath(import.meta.url))))
 const [NAME, PORT_TEXT, ACTION = 'start'] = process.argv.slice(2)
@@ -29,7 +31,8 @@ const { home: HOME_DIR, problem: homeProblem } = benchHome({ name: NAME, root: R
 if (homeProblem) die(`${homeProblem}\n(the bench home must be outside ${ROOT}; export LIVE_E2E_HOME to choose it)`)
 
 const apiKey = process.env.DEEPSEEK_API_KEY ?? ''
-if (!apiKey || apiKey === 'mock-key' || apiKey.length < 16) die('DEEPSEEK_API_KEY is missing, too short, or the mock\'s; a real key is required (never printed)')
+const key = keyReading(apiKey)
+if (!key.usable) die(`DEEPSEEK_API_KEY ${keyProblem(key.reason)}; a real key is required (never printed)`)
 
 const harness = process.env.DSH_HARNESS ?? join(ROOT, 'deepseek-harness')
 if (!existsSync(join(harness, 'package.json'))) die(`no deepseek-harness checkout at ${harness}; set DSH_HARNESS (see CONTRIBUTING.md)`)

dev/live-e2e/key.mjs

@@ -0,0 +1,23 @@
+// One reading of DEEPSEEK_API_KEY for the three places that judge it: the CI preflight that decides
+// whether any step runs (#794), the bench starter, and the run guard. They disagreed — the preflight
+// took any non-empty string, the other two refused a key under 16 characters or the mock's — so a
+// short or mock key published `key=true`, started the bench and failed the night for exactly the
+// reason the skip exists to avoid. A key present but unusable is skipped like a missing one, with
+// its own warning, so the skip never reads as a pass and nobody has to read a red log to find it.
+export const MIN_LENGTH = 16
+export const MOCK_KEY = 'mock-key'
+
+export function keyReading(value) {
+  const key = typeof value === 'string' ? value.trim() : ''
+  if (key === '') return { usable: false, reason: 'missing' }
+  if (key === MOCK_KEY) return { usable: false, reason: 'mock' }
+  if (key.length < MIN_LENGTH) return { usable: false, reason: 'short' }
+  return { usable: true, reason: null }
+}
+
+// The verdict in words, shared so the three readers cannot drift apart in what they say.
+export function keyProblem(reason) {
+  if (reason === 'missing') return 'is missing'
+  if (reason === 'mock') return "is the mock's key"
+  return `is shorter than ${MIN_LENGTH} characters`
+}

dev/live-e2e/report.mjs

@@ -1,5 +1,6 @@
 import { join } from "node:path"
 import { canonicalPath, isInside } from "./integrity.mjs"
+import { keyProblem, keyReading } from "./key.mjs"
 
 // The outcome contract and the report assembly for the #204 live runner. Pure code,
 // covered keyless by test/report.test.mjs.
@@ -30,10 +31,12 @@ export function budgetsFrom(env) {
 
 // Fail-closed checks on the run's inputs. A loopback base URL is the mock bench (or the
 // usage tap, which names the real upstream in LIVE_E2E_TAP_UPSTREAM); anything else
-// without a key, or with the mock's key, is refused rather than run.
+// without a usable key (dev/live-e2e/key.mjs, the reading the preflight and the bench start share)
+// is refused rather than run.
 export function guardRun({ apiKey, baseUrl, tapUpstream, home, userHome, repoRoot, evidence, cwd }) {
   const problems = []
-  if (!apiKey || apiKey === 'mock-key' || apiKey.length < 16) problems.push('DEEPSEEK_API_KEY is missing, too short, or the mock key')
+  const key = keyReading(apiKey)
+  if (!key.usable) problems.push(`DEEPSEEK_API_KEY ${keyProblem(key.reason)}`)
   let host = 'api.deepseek.com'
   let origin = 'default official endpoint'
   if (baseUrl) {

dev/live-e2e/test/key-reading.test.mjs

@@ -0,0 +1,34 @@
+// The suite judges DEEPSEEK_API_KEY in three places: the CI preflight that decides whether any
+// step runs, the bench starter, and the run guard that fails the run closed. Two of them refused
+// a key the third one accepted, so a short or mock key published `key=true`, ran the bench and
+// failed the night for a reason the skip exists to avoid (#794). One reading, three readers.
+import assert from 'node:assert/strict'
+import { readFileSync } from 'node:fs'
+import { test } from 'node:test'
+import { plan } from '../../../.github/scripts/live-e2e-preflight.mjs'
+import { guardRun } from '../report.mjs'
+
+const CASES = [
+  ['', 'a missing key'],
+  [undefined, 'an unset key'],
+  ['   ', 'a blank key'],
+  ['mock-key', "the mock's key"],
+  ['short', 'a short key'],
+  ['a'.repeat(15), 'a key one character under the floor'],
+  ['a'.repeat(16), 'a key exactly at the floor'],
+  ['sk-' + 'a'.repeat(32), 'a real-looking key'],
+]
+
+const keyProblems = (apiKey) => guardRun({ apiKey, baseUrl: undefined }).problems.filter((problem) => problem.startsWith('DEEPSEEK_API_KEY'))
+
+test('the preflight and the run guard accept exactly the same keys', () => {
+  for (const [key, what] of CASES) {
+    assert.equal(plan(key).present, keyProblems(key).length === 0, `the preflight and the run guard disagree about ${what}`)
+  }
+})
+
+test('the bench starter reads the key through the same module, with no threshold of its own', () => {
+  const bench = readFileSync(new URL('../bench.mjs', import.meta.url), 'utf8')
+  assert.doesNotMatch(bench, /\blength\s*</, 'the bench keeps its own length threshold')
+  assert.match(bench, /keyReading|keyState|keyProblem/, 'the bench does not read the shared verdict')
+})

docs/subsystems/live-e2e.md

@@ -39,6 +39,10 @@ suite is one of its columns, not the map's owner.
 - When the key is absent altogether, `live-e2e.yml` skips the six steps that need it and
   warns instead of failing every night (#794). A skipped run proves nothing about the
   suite, and the run summary says exactly that; a failure inside the suite still fails.
+- One reading of the key is shared by the three places that judge it — the preflight, the
+  bench start and the run guard (`dev/live-e2e/key.mjs`): a key under 16 characters or the
+  mock's is skipped with its own warning, never passed to a bench that refuses it. A key
+  long enough but not real is not detected here; only the endpoint can say that (#2214).
 - The mock suites stay separate: this bench never starts the mock model, and the mock
   benches never gain a real key.
 - Evidence is an explicit allow-list that passed the sanitizer (no `token=` URLs, no