fix(design): bound requests and prevent mutation replay
This commit is contained in:
@@ -0,0 +1,94 @@
|
||||
# Task: Diagnose live AI design freeze after HTTP1 package
|
||||
|
||||
## Identity
|
||||
|
||||
- Task ID: 20260819-design-freeze-live-6e2c
|
||||
- Mode: Feature
|
||||
- Branch: codex/20260819-design-freeze-live-6e2c-design-freeze-live
|
||||
- Worktree: D:\mk-design-freeze-live-6e2c
|
||||
- Base commit: 1ba68a9e41ea5eb4cf03bd11ef18ad61023cd069
|
||||
- Owner: codex-root
|
||||
- Status: Completed
|
||||
|
||||
## Scope
|
||||
|
||||
- Inspect the live installed Makelore process after the combined native-login and HTTP/2-disabled diagnostic package froze in AI Design.
|
||||
- Verify package provenance, Chromium transport flags, process health, TCP state, local Host API responsiveness, session-refresh eligibility, Works service health, and the matching request implementation.
|
||||
- Implement the directly authorized fix in the same task: bound finite AI Design and Works session-refresh requests, prevent implicit replay of non-idempotent requests, and add focused regressions.
|
||||
- Build a Windows diagnostic package that retains native password/SMS login and `disable-http2` so this behavior change can be isolated in the user's next smoke test.
|
||||
|
||||
## Intent And Constraints
|
||||
|
||||
- The user explicitly requested direct execution without sub-agents.
|
||||
- Treat the observed 409, global Loading state, and previous `ERR_HTTP2_PROTOCOL_ERROR` as separate signals until live evidence connects them.
|
||||
- Read the encrypted session only to project expiration/activity metadata and to issue bounded read-only requests; never print or persist access/refresh tokens.
|
||||
- Do not replay the Quote PATCH because its idempotency and server-side commit state are not proven.
|
||||
- Remove every temporary harness and isolated Chromium profile before completion.
|
||||
|
||||
## Outcome
|
||||
|
||||
### Confirmed facts
|
||||
|
||||
- The installed `app.asar` SHA-256 exactly matched the combined diagnostic package: `80948DDBBD5F9BDE9B4C75CB40E515E63200AA79582BF16CF88347DECCD864C6`.
|
||||
- The live Network Service process had `--disable-http2`; HTTP/2 was genuinely disabled.
|
||||
- Main, Renderer, GPU, and Network Service remained Windows-responsive. A three-second sample showed zero Main/Renderer CPU growth and only `0.0156` seconds in Network Service, so the process was passively waiting rather than spinning or blocking the UI thread.
|
||||
- While the UI remained stuck, three rounds of 20 concurrent requests to the live Host API all completed with the expected unauthenticated `401` in `47/16/14 ms`. Main's HTTP event loop and listener were not globally blocked.
|
||||
- The stored access token was more than 157 minutes from expiry and outside the 30-second refresh skew. The shared `refreshFlight` was not active for this incident.
|
||||
- With the same authenticated session, read-only Node probes to `/api/auth/me`, Design capabilities, Workspace list, the affected Workspace, and its two Conversations returned `200` in `31-159 ms`. Works, the account, and that Workspace were not globally locked.
|
||||
- A fresh Electron `43.4.0` Network Service with `disable-http2` returned `200` for the same three read-only endpoints in `1764-1853 ms`. HTTP/1.1 itself remained functional.
|
||||
- Product code gives `net.fetch`, its global-fetch fallback, session refresh, design `authorizedFetch`, IPC Host API requests, Workspace/Conversation loads, and Quote repricing no common deadline or abort contract. Their UI cleanup runs only after the Promise settles.
|
||||
- `proxyAwareFetch` catches every Electron `net.fetch` exception and unconditionally replays the same request with global `fetch`. That is unsafe for mutation methods such as Quote `PATCH` when the first request may have committed before its response transport failed.
|
||||
|
||||
### Conclusion
|
||||
|
||||
- Disabling HTTP/2 did not fix the freeze and is not the root cause.
|
||||
- The live symptom is an unbounded in-process network/IPC Promise in the current Makelore network context. The process remains healthy but a request can wait forever, leaving Renderer loading/busy state waiting for `finally`.
|
||||
- The observed `409` is a completed conflict response and cannot itself block the JavaScript thread. It may be a secondary symptom of the unconditional transport fallback replay after an earlier protocol failure, but that specific mutation was not replayed during this diagnostic task.
|
||||
- The user's “one request hangs, then everything stays Loading” report is consistent with missing deadlines/cancellation plus page-level state ownership, not with a dead request thread.
|
||||
|
||||
### Implemented fix
|
||||
|
||||
- Added a reusable deadline primitive that aborts the active transport and still rejects deterministically when a transport ignores `AbortSignal`.
|
||||
- Bounded the complete AI Design JSON request lifecycle to 30 seconds, including token lookup/refresh, response headers, and response-body parsing. The adapter returns a stable `504 DESIGN_WORKSPACE_REQUEST_TIMEOUT` with `AI 设计服务响应超时,请重试`.
|
||||
- Bounded the shared Works access-token refresh to 30 seconds. All coalesced callers settle together, `refreshFlight` is released by its existing `finally`, and a later refresh can retry without clearing a still-valid refresh token.
|
||||
- Restricted Electron-to-Node fetch fallback to safe reads (`GET`, `HEAD`, `OPTIONS`). `POST`, `PATCH`, `PUT`, and `DELETE` transport failures now propagate without implicitly replaying the mutation.
|
||||
- Preserved native password/SMS login and the temporary global `disable-http2` switch in the diagnostic installer so the user can isolate this request-lifetime/replay fix before HTTP/2 policy is revisited.
|
||||
|
||||
## Verification
|
||||
|
||||
- Live installed-package hash comparison — PASS, exact match.
|
||||
- Live process command-line projection — PASS, Network Service had `disable-http2`.
|
||||
- Live process responsiveness/CPU sampling — PASS, responsive passive wait.
|
||||
- Host API concurrency probe — PASS, 60/60 requests completed within 47 ms per round maximum.
|
||||
- Encrypted-session metadata projection — PASS, no refresh window and no token output.
|
||||
- Authenticated read-only Works differential probes — PASS, account/Design/Workspace/Conversation endpoints responsive.
|
||||
- Fresh Electron HTTP/1.1 differential probe — PASS.
|
||||
- Static request-path inspection — confirmed missing deadlines/AbortSignals and unconditional fallback replay.
|
||||
- Temporary scripts, metadata output, and isolated Electron profile — removed.
|
||||
- Red-phase regression run — PASS as evidence: the new mutation-replay, stuck request, and stuck shared-refresh tests all failed against the previous implementation.
|
||||
- Focused unit tests — PASS, 59/59.
|
||||
- TypeScript check — PASS, `pnpm run typecheck`.
|
||||
- Lint check — PASS with zero errors and seven pre-existing unrelated warnings.
|
||||
- Full unit suite — PASS on clean rerun, 184 files and 2190 tests. The first run had one unrelated Windows port-release timing failure; its focused rerun passed before the clean full rerun.
|
||||
- Production compile — PASS, `pnpm run build:vite`.
|
||||
- Windows package build — PASS after the verified prior uv binary and verified Learning Player artifact were reused when GitHub download timed out.
|
||||
- Packaged Learning Player verification — PASS, SHA-256 `748d6d7c74d9d0ba444e0c051a50010f7070e7d6e46442b1e60f1e2be80fd020`.
|
||||
- Packaged `app.asar` marker inspection — PASS for `disable-http2`, `DESIGN_WORKSPACE_REQUEST_TIMEOUT`, and the request-deadline implementation.
|
||||
- Packaged Electron runtime verification — PASS, including Electron `43.4.0`, Node `24.18.1`, OpenCode, Python, uv, npm, and native addons.
|
||||
- Installer SHA-256 — `FB3BDA4F464770E58EA0AA6E989366685976D5FA7B76E6944B92879B423D83D4`.
|
||||
- Packaged `app.asar` SHA-256 — `A752255B43694814CC3D3FB86C75E024D39E67BBF87BE273960A1FA9FA0366A6`.
|
||||
|
||||
## Follow-ups
|
||||
|
||||
- Smoke the original Quote edit/retry/confirm sequence in the diagnostic package. A stalled request should now leave Loading within 30 seconds and show the explicit timeout message; a transport exception must not produce a second Quote PATCH.
|
||||
- After the user confirms the bounded-request fix, restore HTTP/2 in a separate package and repeat the same regression smoke. `disable-http2` remains a diagnostic variable, not the root-cause fix.
|
||||
- Add privacy-safe request lifecycle instrumentation (`route kind`, transport, phase, elapsed time, deadline outcome, correlation id) so a future live freeze identifies the exact pending boundary without DevTools or credentials.
|
||||
|
||||
## Promotion Candidates
|
||||
|
||||
- Target: `.project-docs/20-architecture/data-flow.md` and the AI Design transport boundary.
|
||||
- Proposal: document the implemented bounded AbortSignal/deadline contract for Main-owned finite Works requests and the prohibition on implicit mutation replay without an explicit idempotency contract.
|
||||
- Evidence: this live differential diagnosis, the focused/full regression results, `electron/utils/proxy-fetch.ts`, `electron/services/works-square-session.ts`, `electron/image-workspace/works-square-workspace.ts`, `src/lib/host-api.ts`, and `src/stores/image-workspace.ts`.
|
||||
- Future impact: network failures become terminal and recoverable instead of leaving permanent Loading state or ambiguously duplicating Quote mutations.
|
||||
- Semantic conflicts: none with ADR-001; Workspace/Conversation ownership remains unchanged.
|
||||
- Human confirmation required: no for a bounded bug fix, but permanent HTTP protocol policy should be decided separately after the fixed package is validated.
|
||||
Reference in New Issue
Block a user