fix(web-ai): honor --family and bound assistant DOM reads by the polling deadline - #89
fix(web-ai): honor --family and bound assistant DOM reads by the polling deadline#89dbc-hbin wants to merge 2 commits into
Conversation
…-jun#87) `--family` was advertised by the top-level help, README, the bundled web-ai skill, and the `web_ai_submit_prompt` MCP schema, but the value never reached ChatGPT model selection. Because `parseArgs` runs with `strict: false`, the flag was accepted and silently dropped, so a successful `render` or capability probe looked like proof that the requested family had been enforced. `chatgpt-model.mjs` already implemented family selection and verification; only the wiring was missing. - declare `family` in the send/query parser and carry it into the normalized input - pass `{ family, effort }` into `selectChatGptModel` and the model capability probe so selection and verification actually run - forward `family` in `web_ai_submit_prompt`, and fail closed when it is sent to a provider without a Chat family axis - validate the alias in CLI preflight, before any browser mutation, so an unsupported value such as `gpt-5.6-luna` errors instead of being ignored - document `--family` in `web-ai help`, which previously omitted it Omitting `--family` keeps the existing zero-mutation behavior: the family currently selected in the UI is preserved and no submenu is touched. Tests: new `test/unit/web-ai-chatgpt-family-wiring.test.mjs` asserts at the module boundary that the alias reaches the selector and that an unsupported family fails closed without touching the page; CLI contract tests cover every canonical alias, the invalid alias, non-ChatGPT rejection, and help/parser agreement.
…-jun#88) ChatGPT polling could stay alive with no progress well past `--timeout`. The poll loop only re-checked its deadline at the `while` boundary, then awaited `readAssistantMessages()`, which called `page.evaluate()` with no per-call bound. Playwright's `page.evaluate()` accepts no timeout option, so when assistant message extraction stalled on a very large conversation, control never reached the next deadline check: the process and the session lock stayed alive while the stderr heartbeat went silent. - add `withAssistantReadTimeout()` and `resolveAssistantReadBudgetMs()`; every assistant read is raced against the smaller of the remaining command deadline and a per-read ceiling - bound the `page.evaluate()` path, the locator fallback, and the post-timeout recovery read, which otherwise re-hung the command it exists to rescue - add `readAssistantTextsAfterIndex()`: count turns in-page but serialize only the turns after the baseline, instead of re-serializing the whole conversation on every 500ms tick - keep emitting the poll heartbeat while reads stall, since silence with a live process was the original symptom - report a stalled read distinctly from ongoing generation via an `assistant-dom-read-timeout:<n>` warning on the timeout envelope A read that exceeds its budget now retries on the next tick and, at the deadline, returns the existing recoverable `provider.poll-timeout` envelope, so `poll` and `sessions resume` behavior is unchanged. Exhausting the budget at the loop boundary is treated as the deadline itself, not as a DOM-read failure, so ordinary timeouts are not mislabeled. Tests: new `test/unit/web-ai-assistant-read-deadline.test.mjs` drives `pollWebAi` with a `page.evaluate()` that never resolves and asserts the command still honors its deadline and reports the stall; it also covers the budget helpers and proves historical turns are no longer re-serialized. Verified that removing the bound makes the poll test hang until the runner's own timeout.
79db5d1 to
4ada9cb
Compare
|
Rebased onto Re-verified on top of v0.1.19: All 1448 baseline tests still pass; the delta is the 17 new tests. Also green: The Contract Drift Check workflow shows Note for whichever order you merge in: nothing here touches the new postinstall star-prompt work from v0.1.19. |
WP1 docs-only 사이클. dev 기준 실태 조사와 계획 문서 8종. - 002: PR #89 두 커밋(d5d9475, 4ada9cb)의 dev 대조 판정. #87은 f8e8b9b로 부분 충족(probe/MCP 갭 잔존), #88은 미충족. - 001: devlog _plan 11개 유닛 판정과 00_index.md 드리프트 인벤토리. - 003: A 페이즈 3라운드 FAIL의 근본 원인 분석. #88 방어 범위를 열거로 확정하려는 시도가 매 라운드 새 누락을 낳아, WP3를 경계 인벤토리 확정으로 축소하고 구현은 후속 WP로 분리. 코드 변경 없음.
WP5 A 게이트 6건. 리뷰어가 실제 오류를 셋 잡았다. - 커밋 목록에서 b524453(WP2 계획 보강)이 빠져 16개로 셌다. 실제 17개다(git rev-list --count c7e87c1..HEAD). - 감사 통계가 틀렸다. '리뷰어 4명 16라운드'가 아니라 5명 14라운드 (WP1 9 + WP2 1 + WP3 4)이고 분포는 FAIL 11 / GO-WITH-FIXES 3이다. 출처 없는 '그 외 2라운드'를 지웠다. - main-only 커밋을 6개로 적었으나 실측 5개다. c2 정합: '전수 열거'를 요구하는 criterion을 met으로 두면서 증거는 '표본 36개'라 서로 반박했다. scenario를 표본+예산 계약 기준으로 개정했다 — 전수는 달성 조건이 아니라고 명시했다. 040의 '경계 확정 DONE'도 '부분'으로 고쳤다. diagnostics 배정이 040(DOM 유닛)과 021(artifact 유닛)에서 달랐다. 021 기준으로 통일하고 각 행에 출처를 달았다. 마감 절차: _plan 표에는 이 유닛 행이 애초에 없어서 제거 diff가 적용 불가였다. _fin 행 추가만 남겼다. git mv 후 goalplan capturedEvidence 경로도 갱신하라는 단계를 추가했다 — 증거가 가리키는 곳이 없으면 증거가 아니다. fresh 게이트를 마감 시점에 다시 돌려 기록했다(verify-counts 76, doc-drift 164, typecheck 0). 문서 수정이 카운트를 밀므로 이관 직전 fix:counts 재실행을 절차에 박았다. LOOP-PESSIMIST에 WP2의 죽은 가설 둘 추가(fixture 거짓 양성, guard hole). PR #89 mergeStateStatus UNSTABLE도 인계에 적었다.
유닛을 _fin으로 이관하고 00_index에 closeout 행을 추가했다. WP5 A 게이트 2라운드 잔여 4건도 접었다: - 커밋 목록에 기준점을 달았다(WP5 착수 전 c7e87c1..9c9ea88, 17개). 자기 커밋 해시를 자기 안에 적을 수 없으니 범위를 한정하는 게 맞다. - 감사 통계를 WP1~WP3(5명 14라운드)로 한정하고 WP5 2라운드를 별도 줄로 뒀다. 유닛 전체는 6명 16라운드, FAIL 12 / GO-WITH-FIXES 4. - 040과 goalplan wp3 태스크에 남아 있던 '전수' 표현을 '확인된 표본 전체와 접근 방식'으로 통일했다. c2를 표본 기준으로 고쳐놓고 다른 기록에 전수성이 남아 있으면 같은 모순이다. - diagnostics를 c7(유닛 A)에서 c8(유닛 B)로 옮겼다. 021이 B10/B28을 유닛 B에 배정했는데 criterion이 A를 가리키고 있었다. readActivityState도 '그때 결정'에서 '유닛 A 3번 phase 확정'으로. git mv 후 c1/c2의 capturedEvidence 경로를 _fin으로 갱신했다. 증거가 가리키는 곳이 없으면 증거가 아니다. 최종 게이트 (이관·수정 후 재실행): typecheck exit 0 unit 5파일 129건, integration 3파일 70건 gate:all 16/16, doc-drift 164, verify-counts 76
|
Thanks for this PR — both fixes were real, and both shaped what landed. We could not take the patches directly: dev's assistant reader was already refactored to the split snapshot reader ( |
Fixes #87
Fixes #88
Two related
web-aidefects, one commit each, so they can still be reviewed independently:a2dc3e1—--familynever reached ChatGPT model selection (web-ai CLI silently ignores documented --family #87)79db5d1— assistant DOM reads were not bounded by the polling deadline (pollWebAi can hang past --timeout when assistant DOM evaluation stalls #88)They are combined because both touch
web-ai/chatgpt.mjsand both bump line counts instructure/str_func.md, so merging them separately would need anpm run fix:countsconflict resolution in between. Verified together below.#87 —
--familywas accepted and silently ignored--familyis advertised by the top-level help, README, the bundledweb-aiskill, and theweb_ai_submit_promptMCP schema, but the value never reached model selection. The send/query parser runs withstrict: false, so the flag was swallowed:--family bogus-familyreturned the samerendered/exit 0, so neither output proved anything about the family. Agent workflows could report that GPT-5.6 Sol was explicitly enforced while using whatever family happened to be selected in the browser.web-ai/chatgpt-model.mjsalready implemented family selection and post-click verification (selectChatGptFamily,normalizeChatGptFamilyChoice); only the wiring was missing, so the fix is small.Changes
familyin the send/query parser and carry it into the normalized input{ family, effort }intoselectChatGptModeland the model capability probefamilyinweb_ai_submit_prompt, failing closed when sent to a provider with no Chat family axis--familyinweb-ai help, which omitted it while the top-level help advertised itOmitting
--familyis unchanged: the family currently selected in the UI is preserved and no submenu is touched.An unsupported family also makes the model capability probe report
fail, so a probe result is no longer mistaken for proof that the family was enforced.#88 — polling could outlive
--timeoutpollWebAichecks its deadline only at thewhileboundary, then awaitsreadAssistantMessages(), which calledpage.evaluate()with no per-call bound. Playwright'spage.evaluate()accepts no timeout option (onlylocator.evaluatedoes), so when assistant-message extraction stalled on a large conversation, control never reached the next deadline check. The process and session lock stayed alive while the stderr heartbeat went silent — matching the report wheresessions doctorshowed a valid target, a healthy command-lock heartbeat, and no CDP or login failure.The same function also serialized every historical assistant turn on each 500 ms tick, then sliced off everything before
baseline.assistantCount.Changes
withAssistantReadTimeout()andresolveAssistantReadBudgetMs(); each read races the smaller of the remaining deadline and a per-read ceilingpage.evaluate()path, the locator fallback, and the post-timeout recovery read, which otherwise re-hung the command it exists to rescuereadAssistantTextsAfterIndex(): count turns in-page, serialize only turns after the baselineassistant-dom-read-timeout:<n>on the timeout envelope so a stalled read is distinguishable from ongoing generationA read that exceeds its budget retries on the next tick; at the deadline the existing recoverable
provider.poll-timeoutenvelope is returned, sopollandsessions resumesemantics are unchanged.Two judgment calls worth review:
The trimmed read falls back to the full read whenever it observes no turns, keeping pages that cannot serialize the object argument on the previous behavior.
Verification
Full suite, integrated, with Playwright's pinned Chromium installed so nothing is skipped:
All 1437 baseline tests still pass; the delta is the 17 new tests. Also green:
typecheck:checkjs/typecheck:checkjs-domhave pre-existing errors onmain; I diffed per-file error counts against thea03de27baseline and this branch adds none.New tests
test/unit/web-ai-chatgpt-family-wiring.test.mjs— asserts at the module boundary that the alias reachesselectChatGptModeland the probe, that omitting it keeps the zero-mutation contract, and that an unsupported family fails closed without touching the pagetest/unit/web-ai-assistant-read-deadline.test.mjs— drivespollWebAiwith apage.evaluate()that never resolves and asserts the command still honors its deadline and reports the stall; also covers the budget helpers and proves historical turns are no longer re-serializedBoth were checked against the unfixed code: dropping the
familyargument fails the wiring test, and removing the read bound makes the poll test hang until the runner's own timeout.Live Chrome check
Because the deterministic tests use stub pages, I also ran both read paths against real Chrome over CDP on a ~2 MB, 40-turn conversation:
That is ~1.93 MB of text no longer serialized per 500 ms tick, and the trimmed read still reports the correct total turn count (40).
End-to-end on a real Playwright page whose assistant read never resolves:
Before this change that call did not return at all. The 2.1s overshoot is the intentional post-timeout recovery read under its 2s budget.
Not verified
I did not reproduce the original failure against a live ChatGPT conversation with 40–60 KB assistant answers, so the real-world stall threshold remains unmeasured; the deadline behavior itself is covered deterministically and against real Chrome.
One existing fake page in
web-ai-provider-session.test.mjskeyed its answer sequence off a rawevaluatecall count, so it now keys off the assistant-read argument shape instead. Its assertions are unchanged.