Skip to content

Commit 626ea12

Browse files
fix(web): a planning launch waits for the picker's options before judging the agent
startPlanning() navigated to the Study Session console, then startSession() returned before any fetch with "Select an agent to continue." whenever `this.agent` was still unset — and it is only set once init()'s /api/session/options has resolved. On a cold server a "Plan with architect" click can beat that fetch: the learner lands on the console with no session and a refusal for a choice they were never offered. In CI this was the first e2e test of test_web_plan_architect_journey.py failing on both PR #20 runs (F......... — a navigated page and no POST, then a 20 s timeout) while the nine warm-server tests passed; locally the cold start answered in 60 ms, so it never showed. init() now records the options settlement as _optionsReady (resolved in a finally, never rejecting), and a planning start with no agent awaits it before the check. No agent after the options resolve is still the same named refusal. A focus start is untouched: its Start button is disabled until an agent exists. Proved in a real browser by holding /api/session/options (no continue), clicking, and asserting no POST and no refusal, then releasing and requiring exactly one 201: fails on the old code with "Could not start the architect: Select an agent to continue.", passes with the fix. That scenario is now a permanent journey test; the JS unit suite gains the same race and the no-agent-after-options case (135/135). Journey module 11 passed on ten consecutive runs; one earlier run of the module had a single failure whose identity was not captured before it passed again — recorded, not explained.
1 parent 6b8383b commit 626ea12

3 files changed

Lines changed: 137 additions & 1 deletion

File tree

‎packages/studyloop/src/studyloop/web/static/js/components/session-timer.js‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,13 @@ export function sessionTimer() {
178178
const statePromise = fetch('/api/session/state')
179179
.then((res) => res.ok ? res.json() : {})
180180
.catch(() => ({}));
181+
/* A launch that arrives before the options have resolved (a Plans-view
182+
"Plan with architect" click on a cold server) awaits this before it
183+
judges whether an agent exists -- otherwise it refuses with "Select an
184+
agent" against a picker that simply has not learned its agents yet.
185+
Resolves either way; the fetch's own catch above makes it never reject. */
186+
let markOptionsSettled;
187+
this._optionsReady = new Promise((resolve) => { markOptionsSettled = resolve; });
181188

182189
try {
183190
const options = await optionsPromise;
@@ -197,7 +204,9 @@ export function sessionTimer() {
197204
const firstAvailable = (this.studyOptions.agents || []).find((a) => a.available);
198205
if (firstAvailable) this.agent = firstAvailable.value;
199206
}
200-
} catch { /* enhanced picker unavailable — free-text still works */ }
207+
} catch { /* enhanced picker unavailable — free-text still works */ } finally {
208+
markOptionsSettled();
209+
}
201210

202211
try {
203212
const state = await statePromise;
@@ -275,6 +284,14 @@ export function sessionTimer() {
275284
architect interviews for one, and the server names the session
276285
"Study plan" when none was given — so '' is a valid topic here. */
277286
if (!topic && purpose !== 'planning') return false;
287+
/* A planning launch can arrive from the Plans view before init()'s
288+
options fetch has resolved; the agent is not missing, it is not yet
289+
known. Wait for the picker's own settlement before deciding.
290+
(init() sets _optionsReady on every run; a timer whose init never
291+
ran has nothing to wait for and falls through to the check.) */
292+
if (purpose === 'planning' && !this.agent && this._optionsReady) {
293+
await this._optionsReady;
294+
}
278295
if (!this.agent) {
279296
/* The Start button is disabled without an agent; a Plans-view launch
280297
has no such guard, so refuse here with the picker's own hint. */

‎packages/studyloop/tests/js/plan-architect-launch.test.js‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,3 +436,67 @@ test('liveAgentConsole adopts purpose from /api/session/state on reload', async
436436
assert.equal(con.lastDetail.reattached, true);
437437
assert.match(con.purposeLabel, /planning/i);
438438
});
439+
440+
/* ---------------------------------------------------------------- *
441+
* The cold-server race (CI e2e, PR #20 runs 35214968238 / 35216220593):
442+
* startPlanning() navigated to the console, then startSession() returned
443+
* false before any fetch because `this.agent` was still unset -- init()'s
444+
* /api/session/options had not resolved yet. The learner saw the console
445+
* with "Select an agent to continue." and no session; the journey saw a
446+
* navigated page and no POST. A planning launch must wait for the picker's
447+
* own options before deciding there is no agent.
448+
* ---------------------------------------------------------------- */
449+
450+
test('startPlanning made before the options resolve waits for the agent and still POSTs once', async () => {
451+
let releaseOptions;
452+
const optionsGate = new Promise((resolve) => { releaseOptions = resolve; });
453+
const baseFetch = globalThis.fetch;
454+
globalThis.fetch = async (url, opts) => {
455+
if (String(url).endsWith('/api/session/options')) {
456+
await optionsGate;
457+
return jsonResponse(200, {
458+
agents: [{ value: 'claude', label: 'Claude', available: true }],
459+
topics: [], terminal_engine: {},
460+
});
461+
}
462+
return baseFetch(url, opts);
463+
};
464+
const timer = sessionTimer();
465+
timers.push(timer);
466+
timer.$nextTick = (cb) => cb();
467+
const initDone = timer.init(); // options still in flight: no agent yet
468+
const seen = startEvents();
469+
470+
const launch = timer.startPlanning({ topic: 'SQL window functions' });
471+
await settle();
472+
assert.equal(posts.length, 0, 'nothing to POST until the picker knows its agent');
473+
assert.equal(timer.startError, '', 'must not refuse while the options are still loading');
474+
475+
releaseOptions();
476+
await initDone;
477+
const ok = await launch;
478+
479+
assert.equal(ok, true);
480+
assert.equal(posts.length, 1, 'exactly one POST once the agent is known');
481+
assert.equal(posts[0].purpose, 'planning');
482+
assert.equal(posts[0].agent, 'claude');
483+
assert.equal(seen.length, 1);
484+
assert.deepEqual(navCalls, ['study-session']);
485+
});
486+
487+
test('startPlanning with no agent available after the options resolve still refuses by name', async () => {
488+
const baseFetch = globalThis.fetch;
489+
globalThis.fetch = async (url, opts) => (String(url).endsWith('/api/session/options')
490+
? jsonResponse(200, { agents: [{ value: 'claude', label: 'Claude', available: false }], topics: [] })
491+
: baseFetch(url, opts));
492+
const timer = sessionTimer();
493+
timers.push(timer);
494+
timer.$nextTick = (cb) => cb();
495+
await timer.init();
496+
497+
const ok = await timer.startPlanning({ topic: 'SQL window functions' });
498+
499+
assert.equal(ok, false);
500+
assert.equal(posts.length, 0);
501+
assert.match(timer.startError, /select an agent/i);
502+
});

‎packages/studyloop/tests/test_web_plan_architect_journey.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,3 +595,58 @@ def test_abandoning_a_launch_mid_flight_leaves_no_session_and_no_plan(
595595
assert _visible_purpose_labels(page) == []
596596
# The slot is free: the abandoned session's id is not what a reconnect would find.
597597
assert state.get("last_release", {}).get("study_session_id", study_id) == study_id
598+
599+
600+
# ---------------------------------------------------------------------------
601+
# The cold-server race (PR #20 CI runs 35214968238 / 35216220593, e2e job)
602+
# ---------------------------------------------------------------------------
603+
604+
605+
def test_click_that_beats_the_options_fetch_still_starts_exactly_once(page: Page) -> None:
606+
"""On a cold server the first "Plan with architect" click arrived before the
607+
picker's ``/api/session/options`` had resolved. ``startPlanning()`` had
608+
already navigated to the console, then ``startSession()`` returned before
609+
any fetch with "Select an agent to continue." — the agent was not missing,
610+
it was not yet known. The learner saw the console and no session; the
611+
journey saw a navigated page and no POST, and the first test of this
612+
module failed on both CI runs while the nine warm ones passed.
613+
614+
The options request is HELD here (no ``continue_``) so the click provably
615+
beats it, then released: the launch must wait, not refuse, and then make
616+
exactly one POST that the server answers 201."""
617+
held: list = []
618+
page.route("**/api/session/options", lambda route: held.append(route))
619+
posts: list[dict] = []
620+
621+
def _on_response(response) -> None: # type: ignore[no-untyped-def]
622+
request = response.request
623+
if request.method == "POST" and request.url.endswith("/api/session/start"):
624+
posts.append({"status": response.status, "body": json.loads(request.post_data or "{}")})
625+
626+
page.on("response", _on_response)
627+
_goto_plans(page)
628+
_instrument_starts(page)
629+
assert held, "the options request was never issued, so nothing is being raced"
630+
page.locator('[data-testid="plan-architect-subject"]').fill("SQL window functions")
631+
632+
page.get_by_role("button", name="Plan with architect").click()
633+
page.wait_for_timeout(800)
634+
assert posts == [], "no agent is known yet, so no POST may have been made"
635+
status = page.locator('[data-testid="plan-architect-status"]').inner_text()
636+
assert "select an agent" not in status.lower(), (
637+
f"refused before the options resolved: {status!r}"
638+
)
639+
640+
def _is_start(response) -> bool: # type: ignore[no-untyped-def]
641+
return response.request.method == "POST" and response.url.endswith("/api/session/start")
642+
643+
with page.expect_response(_is_start, timeout=20000):
644+
for route in held:
645+
route.continue_()
646+
647+
page.wait_for_timeout(600)
648+
page.remove_listener("response", _on_response)
649+
assert [p["status"] for p in posts] == [201], posts
650+
assert posts[0]["body"]["purpose"] == "planning"
651+
assert posts[0]["body"]["topic"] == "SQL window functions"
652+
_wait_for_console(page)

0 commit comments

Comments
 (0)