Repository navigation
fix(live-e2e): read DEEPSEEK_API_KEY once for the preflight, the bench and the guard - #2216
Conversation
|
Advisory only: this is a T1 self-test by the team's reviewer, not the repository reviewer's verdict. Coordinates (re-read from the API): head 1. Red-first reproduces the author's numbers exactly. On a clean 2. At the tip. 3. One reading, three consumers — not three copies. 4. Eight inputs agree across all three call sites (probe, runtime values). 5. All three were actually executed, not only unit-tested. Preflight script: 6. The tests have teeth (mutations on the tip, tree clean after each restore). 7. The 11→28 character fixture change really is a fixture. The diff of 8. One observation for the record, not a regression. 9. Also measured earlier for No PR writes were made by the reviewer: no comment, no @, no rerun, no branch push. Posted here on the team's behalf. |
|
@Birfy review request — #2216 at 8a13d76 Ready for review. This tip needs a
What to check: This is the single review request for this tip; I will not post another one. |
… bench and the guard (ThinkFlowLab#2214) The CI preflight published key=true for any non-empty string, while the bench starter and the run guard refused the mock's key and anything under 16 characters. So a short or mock key skipped nothing: the bench started and failed the night for exactly the reason the ThinkFlowLab#794 skip exists to avoid. One module (dev/live-e2e/key.mjs) now returns the verdict and the words for it, and all three readers use it. A key that is present but unusable is skipped with its own warning and its own summary, so the skip never reads as a pass and the reason is visible without reading a red log. A key long enough but not real is still not detected here; only the endpoint can judge that.
|
Verdict: PASS at 8a13d76 Ran at this tip: Review points:
Not verified here: the live suite against the real endpoint (needs a real key, never printed); the change is exercised keyless. |
Written-by: Dev-3
Fixes #2214
Owner: Dev-3
Reviewer: Birfy
Tier: T1
Claim-Id: cd55215b64db48e58e3218f4e5d2a819
Branch: dev/3-live-e2e-key-reading
Base: 77a6b4d
Verified commit: 8a13d76
Tree: 9905d28952b0631112c891e4d6a0fbbfe35a22f4
Files: 9 changed, +115/−18 (the eighth and ninth are new files:
dev/live-e2e/key.mjsand the changelog fragmentchangelog.d/2214-live-e2e-key-reading.changed.md—.changed, carrying(#2214)— which the repository rules require for a change a user can see)The defect
DEEPSEEK_API_KEYwas judged by three rules that disagreed, so a key the suite cannot use skipped nothing and failed the night instead.77a6b4d4.github/scripts/live-e2e-preflight.mjs:29(plan())typeof key === 'string' && key.length > 0key=true; every step that needs the key runsdev/live-e2e/bench.mjs:32!apiKey || apiKey === 'mock-key' || apiKey.length < 16die()— the bench never startsdev/live-e2e/report.mjs:36(guardRun())infra_failed(The live-e2e suite lives at the repository root,
dev/live-e2e/, not undercore/.)Reproduction, measured on the base
guardRun().problemsabcdefgh)key=true, warning 0 bytes, summary 0 bytesDEEPSEEK_API_KEY is missing, too short, or the mock's; a real key is required (never printed)["DEEPSEEK_API_KEY is missing, too short, or the mock key"]mock-keykey=true["…is missing, too short, or the mock key"]'a'.repeat(32))key=trueSo the mis-passed combination is “a key present but under 16 characters, or the mock's”: the preflight green-lights it and the bench fails the night — exactly the red that #794's skip exists to avoid. A key long enough but not real is not judged here at all; only the endpoint can say that.
What changed
dev/live-e2e/key.mjs(new, 23 lines) is the one reading:keyReading(value)→{usable, reason}withreasonone ofmissing(absent, empty or blank),mock,short;keyProblem(reason)gives the shared words (is missing,is the mock's key,is shorter than 16 characters).key=falsefor an unusable key and writes a second warning and summary — “is set but is not a usable key (the mock's key, or shorter than 16 characters)… nothing to run, not because the suite passed” — so a configured-but-wrong secret does not read like an unset one. A blank secret still takes the missing-key path.guardRuncallkeyProbleminstead of carrying their own threshold, so all three print one sentence.key=false+ a 459-byte warning, benchDEEPSEEK_API_KEY is shorter than 16 characters; a real key is required (never printed);mock-key→ preflightkey=false, guardis the mock's key;" "→ the missing-key warning; 32 characters → still passes all three (unchanged).Not touched:
core/client.js,patches/ds-bot/**,plugins/**, anything undercore/.sanitize.mjs'slength >= 8is left alone as agreed on the issue.Tests
Red first, on a clean
77a6b4d4tree with only the tests added:node --test "dev/live-e2e/test/*.test.mjs"— 26 tests, 24 pass, 2 failnot ok 9 - the preflight and the run guard accept exactly the same keys→the preflight and the run guard disagree about a blank key,true !== falsenot ok 10 - the bench starter reads the key through the same module, with no threshold of its own→the bench keeps its own length thresholdnode --test .github/scripts/live-e2e-preflight.test.mjs— 5 tests, 4 pass, 1 failnot ok 5 - a key that is present but unusable skips like a missing one, and says why→a blank secret is the same thing as no secret,+ nullwhere the warning was expectedAfter the change: 26/26 and 5/5.
One existing test changed, fixture only:
plan('sk-live-e2e')and the spawnedDEEPSEEK_API_KEY: 'sk-live-e2e'(11 characters) became'sk-live-e2e-0123456789abcdef'(28), because the shared reading refuses the short one. No assertion was touched.What I ran at
8a13d76dTZ=UTC ARCH_DOCS_BASE=77a6b4d4693b8833f5d3c58b82b9a4b155b8a9c0 npm run verify→ exit 0.github/scriptsarch-docs: OK (9 changed file(s));dev/live-e2e/is an owned path, sodocs/subsystems/live-e2e.mdcarries the invariant in this PR.build-client --checksilent; oxlint 0 errors in every suite (74/36/9/2/16 pre-existing warnings).ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5);ok 10 patches apply cleanly in order;ok core/ equals ds-bot/ + patches/ds-bot/;ok patches/ds-bot/ is what generate writes (generate is a no-op)after twogenerateruns (the second leftgit status --porcelainempty apart from the two bench-onlynode_modulessymlinks).node .github/scripts/changelog.mjs check→44 fragment(s) in changelog.d/ are well formed.Not run
The live suite itself against the real endpoint — it needs a real
DEEPSEEK_API_KEY, which no test may print, and the change is exercised keyless through the preflight, the bench-starter and the guard tests. The smoke test, because nothing undercore/index.jsorcore/src/host/changed.Checkpoint
{
"pr": 2216,
"claim": "Dev-3",
"issue": 2214,
"summary": "The live-e2e preflight accepted any non-empty DEEPSEEK_API_KEY while the bench starter and the run guard refused the mock's key and anything under 16 characters, so an unusable key skipped nothing and failed the night. A new module, dev/live-e2e/key.mjs, now returns one verdict and the words for it, and all three readers use it. An unusable key is skipped with its own warning and summary, so the skip never reads as a pass. A key long enough but not real is still only judgeable by the endpoint.",
"files": [
"dev/live-e2e/key.mjs",
"dev/live-e2e/test/key-reading.test.mjs",
".github/scripts/live-e2e-preflight.mjs",
".github/scripts/live-e2e-preflight.test.mjs",
"dev/live-e2e/bench.mjs",
"dev/live-e2e/report.mjs",
"dev/live-e2e/README.md",
"docs/subsystems/live-e2e.md",
"changelog.d/2214-live-e2e-key-reading.changed.md"
],
"checks": {
"ran": "TZ=UTC ARCH_DOCS_BASE=77a6b4d4 npm run verify (4516/4516); node .github/scripts/changelog.mjs check",
"result": "green",
"at": "8a13d76de26e9d592096fcd989c6fd8d468ec75d"
},
"next": "none \u2014 ready for review"
}