CR Round 2 bucket-(a) fixes for PR #4298:
- agent_server.py: import sys; pass file=sys.stderr to the two observability
print() calls in ForwardedPropsASGIMiddleware (JSON parse failure +
replay_receive() exception path). The accompanying comment claimed stderr
was used "directly" but print() defaults to stdout, so log lines were
interleaving with the SSE response stream on stdout.
- test_forwarded_props.py: the fixture comment claimed "Module-scoped +
autouse" but the decorator is @pytest.fixture(autouse=True) without a
scope= argument (i.e. function-scoped, the default). Function scope is
REQUIRED here — the teardown's sys.modules.pop("agent_server", None) must
run between tests so each one re-binds against fresh stubs. Updated the
comment to match reality and explain why function scope matters; left the
decorator as-is.
Bucket-(b) hygiene applied while in the area (tests still pass):
- Replaced the inline `assert isinstance(forwarded, dict)` in the test-side
reference splice with the `if not isinstance: return body` pattern that
mirrors the production-side narrowing in agent_server.py
(assert is stripped under `python -O`).
- Tightened test_real_agent_server_streams_post_root_without_runtimeerror's
SSE assertions: parse the STATE event JSON and assert
state.inputs.tone == "casual" + the agent_config_guidance prefix shape,
rather than substring-checking the whole response body (which would pass
even if props landed in the wrong key).
Verification: 102/102 pytest passing. Standalone runs of
test_post_without_forwarded_props_passes_through_unchanged and
test_health_endpoint_short_circuits both pass. Ruff clean on edited lines
(pre-existing E402 in the load_dotenv ordering block intentionally left
alone). Pyright clean on edited surface.
Address bucket-(a) review findings on the raw ASGI middleware:
* replay_receive: explicitly catch and re-raise asyncio.CancelledError so
task cancellation propagates correctly (don't rely on the BaseException
hierarchy). Narrow the bare `except Exception` to log unexpected
transport errors before treating them as disconnect — silent swallow
was masking real bugs.
* Mid-buffer http.disconnect: replay buffered chunks as a single
http.request to the inner ASGI app BEFORE forwarding the disconnect.
Previously the inner app saw a clean disconnect with no body even
when partial chunks had arrived, hiding what the client actually sent.
* JSON parse failure: log a stderr breadcrumb with method/path context.
Behavior unchanged (still falls through to verbatim replay), but
malformed bodies are now observable instead of silently invisible.
Bucket-(b) cleanups picked up while editing:
* _splice_forwarded_props docstring updated to note in-place mutation.
* Replace the strippable `assert isinstance(forwarded, dict)` with a
defensive `if not isinstance(...)` so `python -O` doesn't lose the
guard.
* Use `k.lower() == b"content-type"` for parity with the content-length
filter and to be defensive against non-spec ASGI servers.
* Drop the empty-string defensive case from the path tuple — ASGI's
`path` is always at least "/".
Tests:
* test_replay_receive_propagates_cancellederror — drives the middleware
with a receive() that raises CancelledError and asserts it propagates.
* test_replay_receive_explicitly_handles_cancellederror — source-level
pin that `except asyncio.CancelledError` is present.
* test_replay_disconnect_delivers_buffered_chunks_before_disconnect —
asserts the inner ASGI app sees buffered chunks BEFORE the
http.disconnect when disconnect arrives mid-stream.
Full python suite: 102 passed (was 99 + 3 new). pyright clean on
agent_server.py modulo the pre-existing missing ag_ui_crewai stub.
Two correctness issues with the bucket-(a) tests CR called out:
1. The streaming regression test had a no-op pre-fix path: it built
`_OldForwardedPropsMiddleware` via `_build_app_with_real_middleware` but
then never called `app.add_middleware(OldMW)` — `if OldMW is not None:
pass` was dead. The "RED on master" claim was false; the regression
pin lives entirely in the structural assertion that grep-checks
`class ForwardedPropsMiddleware(BaseHTTPMiddleware` is gone from
agent_server.py. Verified the pin trips RED if the racy shape
reappears.
Deleted: `_build_app_with_real_middleware`, `_make_streaming_app`,
`_import_agent_server_helpers` (raised NotImplementedError, never
called), and the dead `if OldMW is not None: pass` block. Rewrote
the streaming test docstring to describe the actual ASGI middleware
fix (not the false "splice moved into route handler" narrative).
2. `test_health_endpoint_short_circuits` and
`test_post_without_forwarded_props_passes_through_unchanged`
depended on `test_real_agent_server_*` running first to install
`ag_ui_crewai` + `agents.*` stubs in `sys.modules`. Running either
alone (e.g. via `pytest -k <name>`, pytest-xdist, pytest-randomly)
failed with `ModuleNotFoundError: No module named 'ag_ui_crewai'`.
Moved stub setup into a module-scoped autouse fixture
(`_stub_agent_server_deps`). The fixture also drops the cached
`agent_server` from `sys.modules` so each test re-imports against
our stubs (not whatever sibling test installed first).
Verified: 99/99 pytest pass, standalone `pytest -k
test_post_without_forwarded_props_passes_through_unchanged` passes,
ruff clean, pyright 0 errors, structural pin trips on simulated
pre-fix source.
Add a hard env-var gate at the top of e2e-parity.ts::run. When
D6_ENABLED is anything other than the literal string "true"
(case-insensitive) the driver short-circuits with an aggregate green
"D6 disabled" signal and emits no per-feature side rows.
Why: D5 ships immediately (auto-fires after merge) but D6 is deferred
until calibration is ready. The previous soft gate (throw on unset
D6_MODE) flapped red on every tick — wrong signal for "intentionally
deferred". Green-with-note matches existing skipped semantics.
The gate supersedes the cron schedule: even with the schedule firing,
the driver no-ops unless an operator flips the flag on the
showcase-ops Railway service. Once enabled, all existing behaviour
applies (D6_MODE / D6_TARGET_INTEGRATION still required for
on-demand).
Tests cover unset / "true" / "TRUE" / "false" / "" cases with
mock call-count assertions proving collaborators are not invoked
when disabled. README and YAML config updated to document the flag.
Pyright was unable to narrow `inputs` after the
`state.get("inputs") if isinstance(state.get("inputs"), dict) else {}` pattern
(two distinct expressions, no narrowing across them) — it inferred
`Unknown | dict | None` and then mismatched `dict()` overloads, which cascaded
into bogus bytes-key errors on `inputs[...] = ...` assignments. Pull the
narrowing into a single named binding and add explicit `dict[str, Any]`
annotations on `state` and `inputs` so the dict-mutation calls type-check.
Also add an `assert isinstance(forwarded, dict)` after `_has_agent_config_props`
returns true (the helper already guarantees it, but Pyright can't see the
contract through the bool return).
In tests, replace direct attribute assignment on `types.ModuleType` instances
with `setattr()` — the dynamic stubbing is the test's whole purpose, and
Pyright correctly flags the static attribute writes as
`reportAttributeAccessIssue`. `setattr()` is the standard escape hatch for
this pattern.
No behavior change. 99/99 tests still green.
Small, collapsible disclosure panel above StatsBar on the Cells tab with
plain-English definitions of the D0–D6 chip layers. Default collapsed;
open/closed state persists across navigations via
localStorage["chip-explainer-open"]. Footer link points to the Notion
plain-English page for deeper detail.
The Cells tab is consulted dozens of times a day by operators who already
know what the chips mean; the explainer is for first-time viewers and
rare refreshers, so the trigger is a small text button and the default
is collapsed to keep the layout clean.
The chat regression on crewai-crews surfaced as AG-UI clients receiving
RUN_STARTED and then the SSE stream aborting (RUN_ERROR: INCOMPLETE_STREAM,
no TEXT_MESSAGE_* / RUN_FINISHED ever emitted). Production logs from the
Railway crewai-crews service confirmed the abort was triggered by:
RuntimeError: Unexpected message received: http.request
raised from `starlette/middleware/base.py:55` mid-stream.
Root cause: ForwardedPropsMiddleware (introduced as part of the agent-config
demo plumbing) was a BaseHTTPMiddleware subclass that called
`await request.body()` to inspect / mutate the JSON body, then reinstalled
`request._receive` with a one-shot replay. Stacked under HealthMiddleware
(also a BaseHTTPMiddleware), that body-replay raced with Starlette's inner
anyio TaskGroup `wrapped_receive`. The middleware fired its bailout check
(`_has_agent_config_props`) only AFTER `request.body()` had already consumed
the receive queue — so the bug fired on every POST, regardless of whether
forwardedProps was present.
Fix: drop ForwardedPropsMiddleware and replace it with a raw ASGI middleware
(`ForwardedPropsASGIMiddleware`). The raw form owns the receive stream from
the outset and replays buffered bytes via a single fresh receive — no
`_receive` reinstallation, no BaseHTTPMiddleware machinery to race against.
The forwardedProps -> state.inputs splice logic is unchanged (extracted into
a pure dict-in / dict-out `_splice_forwarded_props` helper) so the
agent-config demo's tone / expertise / responseLength contract is preserved.
TDD: tests/python/test_forwarded_props.py reproduces the bug at the ASGI
layer (no LLM, no crewai required) and pins five contracts:
- streaming POST through the middleware stack completes without RuntimeError
- the racy `class ForwardedPropsMiddleware(BaseHTTPMiddleware` is gone
- end-to-end import of agent_server + POST to / streams cleanly
- /health continues to short-circuit
- non-agent-config POSTs pass through unchanged
Pre-fix RED: 3 failures, including the exact RuntimeError from production
logs surfaced via Starlette TestClient. Post-fix GREEN: 99 / 99 pass.
R4-A.2: ProbeInvokerDeps.schedulerId is now required (no `?? cfg.id` fallback).
The earlier optional+default path silently re-introduced the R3-A.2 bug for any
caller that forgot to supply the prefixed scheduler id — tracker registration
became a silent no-op against the live scheduler. Required-at-typecheck
surfaces missing-prefix mistakes immediately. All 30 test invoker
constructions updated to pass schedulerId: cfg.id.
R4-A.7: outer fan-out catch now synthesizes a failed-tile + bumps `failed`
+ flips runState to "failed" instead of just logging. Pre-fix, if an
invariant inside the inner pool ever broke, the run would persist as
state="completed" with failed:0 while a real defect occurred — fake-green.
Post-fix, the synthetic tile is keyed `<probeId>:__internal_invariant__`,
written through the writer like any other failure, and the RunSummary
preserves total === passed + failed.
Adds covering tests for both: the typecheck contract via @ts-expect-error
on a fenced call, and the runtime fail-loud behavior via a fake scheduler
whose tracker.start throws (the only path that escapes Promise.all into
the outer catch).
- R4-A.1: response `scope` is now null when no filter was provided in the
request body, vs the actual array (possibly empty) when filter.slugs was
sent. Operators can distinguish "no filter" from "empty scope" in audit
logs.
- R4-A.4: post-read body cap now compares Buffer.byteLength(raw, "utf8")
against TRIGGER_BODY_LIMIT_BYTES instead of raw.length. UTF-16 char
counts let multibyte payloads (emoji = 4 bytes / 2 code units) slip
past a byte-denominated cap.
- R4-A.5: replaced the post-read length check with Hono's bodyLimit
middleware. Chunked uploads that omit (or lie about) Content-Length
now abort the body read at limit+small via a streaming reader, rather
than buffering the full body before checking. BodyLimitError surfaces
as 413 on the read; the post-read byte check stays as defense in depth.
- R4-A.6: rate-limit rollback now fires on ALL non-success paths from
scheduler.trigger, not just InflightConflictError. A transient 5xx no
longer locks the operator out of the 5-min window. Added structured
console.error("probes.trigger-unexpected", ...) before the rethrow so
500s are diagnosable from logs alone.
Tests: 6 new red-green tests covering scope null vs [], UTF-8 byte cap,
streaming early bail, and generic-error rollback. Updated the existing
"queued envelope" test to assert scope: null (was scope: []).
R4-B.1: useProbes effect now calls setData(null) on dep change to mirror
useProbeDetail's CR-B1.4 behavior. Without this, the dashboard renders
stale list data from the previous baseUrl until the new fetch resolves.
R4-B.6: useTriggerProbe checks controller.signal.aborted after the await
resolves. If a back-to-back trigger aborted the controller after the
fetch already returned, honor the R3-C.1 supersession contract and
resolve to null instead of returning the stale TriggerResponse.
serve() from @hono/node-server returns the http.Server synchronously, but
the bind happens via server.listen() which emits 'error' asynchronously
for conditions like EADDRINUSE. The R2-B.2 try/catch only caught
synchronous throws, so a real bind failure resolved boot() successfully
with an orphaned scheduler still ticking.
Race 'listening' vs 'error' on the returned server. On 'error', flip
schedulerRunning false, fire scheduler.stop() as a separate cleanup
chain (so the rejection isn't gated on stop), and reject boot() with
the original error. Guard the 'already listening' edge case so a sync
bind that beats listener attachment doesn't hang.
Adds covering test that mocks serve() to return an EventEmitter-like
stub which emits 'error' on next tick — boot() must reject and
scheduler.stop() must be called.
Two related test-isolation fixes for `src/orchestrator.test.ts`:
1. Bump global `hookTimeout` from 10s default to 60s. Several integration
tests boot a real Hono server, chokidar watcher, scheduler, and PB
client in beforeEach and tear them down in afterEach. On macOS + CI
runners the teardown can spike well past the default 10s ceiling
(chokidar.close on a deleted tempdir + http.Server.close + scheduler
settle), producing cascading "Hook timed out in 10000ms" failures on
/health-wiring, OPS_TRIGGER_TOKEN, and probe-unregister blocks. The
R21-a SIGHUP-reload test deletes its configDir mid-test and chokidar's
watcher.close() on a vanished path can hang for ~30s. 60s absorbs the
variance without masking real hangs (a stuck stop() blocks for minutes,
not seconds).
2. R21-a SIGHUP test: replace the fixed `setTimeout(100)` wait with a
polling loop that checks for the synthetic `<sighup>` emission with a
15s ceiling. Empirically the SIGHUP reload chain
(loader.load → loadDefaults readFile ENOENT → fs.readdir ENOENT →
reloadRules .catch → bus.emit) settles 2-5s later on macOS under load
— the prior 100ms budget produced a deterministic false-fail even
though the production path was correct.
Result: full showcase-ops suite — 941/941 tests pass; orchestrator.test.ts
run twice consecutively without flake. Typecheck clean.
Test names and section banners carry CR-round bookkeeping markers
((A1), (A6), (A11), (E1), (E2), F1.B/C/D/E/F) that leaked into the
suite during the R4 fix cycle. Those markers are useful while the
round is open but become noise once the work lands — operators
running `vitest -t` against the master suite shouldn't have to
remember which bucket a fix came from.
Strip all CR-bucket prefixes from `it()` names, describe-block names,
and section-divider comments. Test contents and assertions are
unchanged; this is a pure rename pass for long-term hygiene.
If `probeLoader.watch(...)` threw synchronously (e.g. an underlying
fs.watch ENOENT path), `unwatchProbes` was never bound, so a
subsequent `stop()` invocation hit `ReferenceError: unwatchProbes is
not defined` and orphaned the engine + scheduler.
Initialize `unwatchProbes` as a no-op BEFORE the watch() call, then
overwrite if registration succeeds. A failed init is logged via
`logErrorWithStack` rather than crashing the boot path.
R5-G4 D7.
Pre-fix, if both SHARED_SECRET and SHARED_SECRET_PREV were unset/empty,
webhookSecrets resolved to `[]` and webhook auth on the deploy.result
endpoint was silently disabled — anyone who knew the URL could POST.
Mirror the POCKETBASE_URL fail-loud pattern: in production, missing
webhook secret is a deploy bug that must surface immediately, so throw
FATAL-CONFIG. In dev/test emit an info-level log so developers see the
bypass without crashing local boot.
R5-G4 D5.
Comment-only cleanups (no behaviour change):
A3: rephrase the logErrorWithStack JSDoc so the "every time" wording
no longer contradicts the implementation (string-thrown values have
no stack to preserve, so the full-stack debug emission is conditional).
A4: collapse two duplicate sort-gate paragraphs into one, mention
"head-of-line blocking under bounded concurrency" so the reasoning
for why the sort exists is in one place rather than scattered.
A5: replace the dismissive "listener-cleanup is not a real concern"
finally-block comment with an honest acknowledgement that the listener-
leak window is bounded by GC of the AbortController after resolveInputs
returns. Documents the tradeoff rather than pretending it doesn't
exist.
A10 (schema.ts): drop the "the 845/847 unit-suite passed" reference
that pinned the comment to one specific test count. Replace with a
generic statement that the loader unit tests use stubs not real YAMLs,
so the gap only surfaces in integration.
The orchestrator's inline `gql` in `createRailwayAdapter` diverged from
the discovery source's `makeGql` on two axes:
1. Non-JSON response bodies (HTML edge-proxy 5xx pages) surfaced as a
raw `SyntaxError` from `res.json()` instead of a schema-class
classification — operators reading the log stream couldn't tell
schema drift from a bad gateway.
2. Any non-empty `errors[]` threw even when `data` was present;
`makeGql` returns the partial payload and logs the errors. Two
callers running the same query saw different exception shapes
depending on whether they hit the inline gql or the discovery one.
Fix: export `makeGql` and `RAILWAY_GRAPHQL_ENDPOINT` from
`probes/discovery/railway-services.ts` and route `createRailwayAdapter`
through it, so error taxonomy (Auth / Backend / Schema / Transport
class hierarchy) is uniform across both consumers.
While here, drop the in-adapter `cachedServices` cache (D6). The
aimock-wiring probe always calls `listServices` first, and per-tick
refetch costs one round-trip but ensures renamed/added services
surface on the next cron tick without orchestrator restart.
`createRailwayAdapter` is also exported now and accepts an optional
`{ fetchImpl }` dep so unit tests can drive arbitrary HTTP behaviour.
R5-G4 D2/D3/D6.
Discovery returning zero records (everything filtered out, registry
empty, etc.) was silent — no signal to operators that the tick existed
but produced zero writes. Add a `probe.no-inputs` info log carrying
`probeId` + `kind` so log-aggregation can correlate "no signal"
against "discovery returned empty". Behaviour is unchanged — zero
inputs still mean zero writes — this is observability only.
Also drop the dead `if (!item) break;` defensive guard in the worker
loop. The loop precondition `cursor < inputs.length` guarantees
`inputs[idx]` is defined; the guard was a TypeScript-strictness
artifact, not a real invariant. Replace with `inputs[idx]!` plus a
comment documenting the loop-precondition invariant.
TDD red-green test: discovery source returns `[]`, asserts zero
writes plus exactly one structured `probe.no-inputs` info log.
Reorganises the cost-budget paragraph in config/probes/e2e-demos.yml
to cleanly distinguish:
- per-service average (~32 demos)
- the largest single service (~38 demos, wall-clock dominator)
- aggregate page loads per tick (~544)
and explicitly calls out the per-demo wall-clock bound enforced by
runDemo()'s single deadline across goto + fallback selectors. The
20-min outer cap and the worst-case ~19 min math now follow from
that bound directly, matching the driver's actual behaviour.
Driver throws can carry multi-MB Playwright stack traces or browser
console dumps. Without truncation those propagate untouched into PB
rows and Slack alerts, blowing past render budgets and making the
dashboard unreadable.
Add a 1200-char `SYNTHETIC_ERROR_MSG_BUDGET` (matching the budget the
e2e drivers themselves self-truncate to) and pipe the synthetic
errorDesc through `truncateUtf8` from `render/filters.js`. Same UTF-8-
safe truncation the renderer uses, so the synthetic path matches what
drivers produce.
TDD red-green test throws a 50KB error and asserts errorDesc <= 1200.
Adds 10 new tests in src/probes/drivers/e2e-demos.test.ts covering the
behaviors hardened in the driver:
- registry-read-failed at info on ENOENT (downgraded from warn)
- registry-parse-failed at warn on corrupt JSON
- registry-shape-invalid at warn when root is not a plain object
- non-abort error classified as driver-error even if abort races
- abort mid-selector-loop short-circuits without walking all 5 selectors
- empty-string route emits red config-invalid side row
- slow writer does not block abort fan-out (fire-and-forget side emit)
- writer-missing warn deduped to at-most-once per run
- per-demo wall-clock bounded to pageTimeoutMs across goto + selectors
- synthetic resolver-error side row when custom resolver throws
Also introduces mkSpyLogger() / mkCtxWithLogger() helpers and an
imported Logger type to keep the new structured-logging assertions
type-safe.
When the railway-services discovery source can't load registry.json (or
every record otherwise has zero demos), the existing sort still runs
its tie-break on `key` ascending — silently re-ordering discovery's
natural emission order without operator signal. Skip the sort entirely
in that case and emit a structured `probe.e2e-demos.sort-no-demos`
warn with the probe id, input count, and a hint pointing operators at
the most likely cause (registry.json missing/corrupt).
TDD red-green test exercises three records in non-alphabetic order
(zulu/alpha/mike) and asserts dispatch preserves enumeration order
plus exactly one structured warn fires.
Pre-fix `diffProbeSchedules` wrote `process.env.E2E_DEMOS_TIMEOUT_MS = String(cfg.timeout_ms)`
directly. Three problems with that approach:
1. Stale across YAML reloads — when a reload dropped timeout_ms, the
env var stayed set and leaked into every subsequent probe tick.
2. Last-write-wins silently across multiple e2e_demos configs.
3. Mutating shared global state from a diff function broke parallel
test isolation.
The fix: a pure `envForCfg(cfg, baseEnv)` overlay function. The
orchestrator now snapshots process.env once at the top of the diff
loop, and each probe gets a per-cfg overlay handed to its invoker.
process.env is never mutated.
R5-G4 D1.
Two related observability fixes:
A2: stamp `errName: "TimeoutError"` on the synthetic signal returned
by both timeout-path branches (setTimeout callback and the post-race
`if (timedOut)` branch). Operators triaging "timeout vs. driver-error"
no longer have to parse free-form errorDesc — the discriminator is on
the signal directly.
A8: tighten the object-shape gate in resolveInputs to reject `null`
explicitly (previous `record &&` truthiness check let null records
fall through silently). Else-branch now emits a single
`probe.discovery-record-non-object` warn for every non-object record
(array/null/number/string/boolean) with a `recordKind` discriminator.
Previously only arrays warned; null and primitives slipped through
silently.
TDD red-green tests for both: timeout signal carries
errName='TimeoutError'; non-object discovery records (null, number,
string, boolean) each emit a structured warn with the right kind.
Several CR R5 G3 fixes to drivers/e2e-demos.ts, all in the same file
because the changes touch overlapping regions:
C1: drop redundant path.resolve("/app/data/registry.json") wrap. No-op
for an absolute path; mirror railway-services.ts's plain string fallback
so the constant stays greppable across the two readers.
C3: replace empty catch {} in defaultDemosResolver with structured
read/parse/shape error logging. Mirrors loadDemosMap in
railway-services.ts: ENOENT → info (steady-state in dev), other read
errors → warn, JSON parse errors → warn, non-object root → warn. A
corrupt or missing registry now emits a distinct log key per failure
mode instead of silently flipping every e2e_demos cell green forever.
C4: document the per-run cache lifetime. The factory is invoked inside
run() so the cache survives within one tick (avoids 17 redundant reads)
but is discarded across ticks (so a registry update mid-day takes
effect on the next tick without driver restart). Hoisting to module
scope would make the registry stale forever.
C5: classify run errors via err.name === "AbortError" instead of
reading abortSignal.aborted after the catch settles. The prior approach
let a concurrent abort flip the flag mid-throw and misclassify a real
TypeError / TargetClosedError as "abort".
C6: log page/context teardown failures at debug instead of swallowing
silently. Stable log keys (page-close-failed, context-close-failed) so
operators can distinguish the two cases.
C7: check abortSignal.aborted at the top of every selector-loop
iteration so a cap that fires mid-loop bails promptly rather than
walking all 5 selectors at pageTimeoutMs each.
C8: treat empty-string route as a config mistake. Surface a red row
with errorClass="config-invalid" and errorDesc="route is empty
string" instead of falling into the informational-cell green path.
C9: void sideEmit on the abort branch instead of awaiting. A slow /
hung writer was defeating the prompt-shutdown contract for remaining
demos.
C10: dedup writer-missing warn via a per-run closure. N demos with no
writer wired used to produce N warns; now exactly one per run.
C11: bound per-demo wall-clock to pageTimeoutMs across goto + all
selector waits via a shared deadline. Worst case was 6*pageTimeoutMs
per demo; now strictly bounded.
C12: emit a synthetic e2e:<slug>/__resolver red row with
errorClass="resolver-error" when a custom resolver throws. Prior
behaviour fell through to demos: [] which masquerades as a green
aggregate; the synthetic row surfaces the configuration mistake on
the dashboard distinctly from an empty registry.
C2: align the abort-branch comment with the actual continue (not
break) semantics — every remaining demo gets a red side row.
A driver rejection BEFORE the timeout fires reaches both the detached
`.catch` observer and the outer `try/catch` which logs probe.run-failed.
Without this guard we double-log on every normal rejection. Add an
`if (!timedOut) return;` so only the true late-rejection path (timeout
won the race, driver rejected afterwards) emits the debug log.
Adds a TDD red-green test that asserts exactly one probe.run-failed log
and zero probe.driver-late-rejection logs on a normal driver throw.
The docstring for `selectorCascade` says "Race a list of selectors and
return the first one to resolve", but the implementation iterated
selectors sequentially with `await page.waitForSelector` — on a full
miss with 4 selectors at 3s each, total wait was 12s instead of 3s.
Convert to `Promise.any` over concurrent `page.waitForSelector` probes;
all selectors are dispatched in the same tick and whichever resolves
first wins. On full miss, throw with a consolidated reason listing
each tried selector's failure detail (preserves the existing label-
based error contract for callers).
Add a covering test that asserts the helper dispatches all
waitForSelector calls within ~50ms of each other (would fail at >50ms
spread under the previous sequential implementation, which would
serialize 50ms-rejecting probes). Plus a fast-vs-slow probe test
that asserts the FAST resolution wins regardless of cascade order
(the race semantic the docstring promises).
The existing `pickTimeSlot` and `approveOrDeny` callers continue to
work unchanged — `Promise.any` returns the SAME selector value the
sequential loop returned (the first to resolve), and the caller-side
selector composition (`${cardSelector} ${slotEntry}`) is unaffected.
The fenced ts block had `launchBrowser: /* see scripts/d6-capture-references.ts for the wiring */,`
which is invalid TS — a comment cannot be a property value. Replace the
broken example with a `yourLauncher` placeholder and a prose pointer to
the canonical wiring in scripts/d6-capture-references.ts.
Also drop the duplicate exit-code paragraph that lived under the
programmatic helper section — exit codes are a CLI concern only, and
the CLI section already states them on line 59.
R3-A.1: filter.slugs path now retains every input with a preError set so
discovery-time key_template failures still surface as synthetic-error
results when a `--slugs foo` manual trigger runs. Pre-fix the resolver's
synthetic preError keys (e.g. `<probeId>:invalid-key-template:N`) never
matched operator-supplied slugs and were silently dropped, hiding the
exact misconfigurations operators were investigating.
R3-A.2: invoker now reads `deps.schedulerId` (defaulting to `cfg.id` for
test fixtures) and uses it for `scheduler.getEntry` and
`setEntryTracker`. Orchestrator passes the prefixed `probe:<cfg.id>` it
registers under. Pre-fix the invoker passed the bare `cfg.id` while the
orchestrator registered with the `probe:` prefix, so `getEntry` returned
undefined and `setEntryTracker` was a silent no-op against the live
scheduler — `/api/probes` never surfaced inflight tracker data for
cron-tick runs and the cleanup setEntryTracker(null) was a no-op too.
R3-A.3: emit a structured `probe.run-row-orphan-risk` warn alongside the
existing `probe.run-writer-start-failed` error when runWriter.start
throws. The PB-side create may have succeeded before the response was
lost, leaving an orphan `running` row with no caller holding the id.
The warn is the observability surface operators grep for; the run
continues normally because probe_runs is best-effort observability.
github-actions[bot]