fix: isolate concurrent OpenCode chat runs
This commit is contained in:
@@ -0,0 +1,103 @@
|
||||
# Task: Fix concurrent chat Agent registry stall
|
||||
|
||||
## Identity
|
||||
|
||||
- Task ID: 20260817-multichat-runtime-fix-f3a91c
|
||||
- Mode: Feature
|
||||
- Branch: codex/20260817-multichat-runtime-fix-f3a91c-multichat-runtime-fix
|
||||
- Worktree: D:\Datas\OthersProjects\makelore-multichat-runtime-fix-f3a91c
|
||||
- Base commit: 7e8d9e38114158992c03e589de32535274796d04
|
||||
- Owner: codex-root
|
||||
- Status: Ready for integration
|
||||
|
||||
## Scope
|
||||
|
||||
- Add a Main-owned project Agent readiness barrier before project-scoped OpenCode prompts.
|
||||
- Track the generated Makelore Agent set by runtime generation and content fingerprint so a same-id content change cannot be mistaken for an applied runtime update.
|
||||
- Verify the selected Agent against the live OpenCode registry before `prompt_async`; return a typed terminal response without sending when the runtime registry is stale.
|
||||
- Make Renderer prompt startup acknowledgement session-scoped and bounded: only explicit busy state, assistant output, or a typed terminal event confirms/ends startup; a persisted user message alone does not.
|
||||
- Add focused regression coverage for concurrent sessions, stale/new Agent handling, runtime generation rollover, managed-file preservation, and per-session startup failure cleanup.
|
||||
- Do not patch or fork the bundled OpenCode executable in this task.
|
||||
|
||||
## Intent And Constraints
|
||||
|
||||
- Preserve a running Session A while Session B is submitted; no application-wide reply-duration lock.
|
||||
- OpenCode 1.18.9 exposes no authoritative whole-instance quiescence oracle. Automatic `/instance/dispose`, reload, or runtime restart must not be used as Agent refresh, even when `/session/status` appears idle.
|
||||
- A prompt-start acknowledgement must not be inferred from the user prompt merely appearing in session history. If no explicit busy/assistant/error signal appears within the bounded window, terminalize only that session as `SESSION_START_UNCONFIRMED`; do not replay automatically.
|
||||
- Only a typed preflight failure that guarantees `prompt_async` was never called may retain a cancelable same-process pending intent. Network failures and unconfirmed starts are terminal and must never be auto-replayed.
|
||||
- Generated Agent readiness is bound to `{runtimeGeneration, desiredFingerprint, appliedFingerprint}`. `GET /agent` verifies live id presence but cannot prove same-id content freshness.
|
||||
- The managed fingerprint covers relative path plus generated content hash. Preserve non-Makelore files in `.opencode/agent`; remove only files known to have been generated by Makelore configuration.
|
||||
- Keep Renderer backend access through existing Host API modules and keep Electron Main responsible for runtime/configuration synchronization.
|
||||
- Avoid paid provider calls in automated verification; use unit/integration doubles and perform only local-runtime smoke checks when available.
|
||||
- Completion requires focused tests, typecheck, lint, full unit tests, `build:vite`, and a final read-only Sol reviewer PASS.
|
||||
|
||||
## Outcome
|
||||
|
||||
- Added a Main-owned project Agent readiness barrier keyed by runtime generation and the generated Agent manifest fingerprint. A prompt is rejected before `prompt_async` with typed `OPENCODE_AGENT_REGISTRY_PENDING` when the live registry cannot prove the desired Agent set was loaded for the active runtime generation.
|
||||
- Runtime generations carry explicit `unknown` / `starting` / `fresh` / `attached` provenance. Only a fresh owned-runtime generation may bind the desired fingerprint as applied; attached and unknown runtimes fail closed even when the live registry contains the same Agent id.
|
||||
- Preserved custom and user-modified `.opencode/agent` files by reading the previous project config without materialization side effects before retirement checks. Only retired Makelore-managed files whose on-disk content still matches the previously generated content are removed.
|
||||
- A prompt that finds stale provider/runtime configuration now returns typed `OPENCODE_RUNTIME_CONFIG_PENDING` with `promptSent:false` before sending. The prompt path never restarts the shared runtime; explicit manual lifecycle actions retain their existing behavior.
|
||||
- Legacy/direct Works credential import supports a deferred runtime-refresh mode. Prompt submission persists a refreshed credential but returns the same typed pending response when the active runtime must reload; it neither restarts nor calls `prompt_async`. The requirement is remembered for the active runtime generation until a fresh generation appears.
|
||||
- Automatic Works model synchronization from ordinary messages, project commands, and context summarization now explicitly uses deferred refresh. If the active runtime must reload, Renderer preserves the draft, shows a manual-restart instruction, and does not submit the execution request.
|
||||
- Runtime configuration freshness is latched by manager identity and runtime generation for direct API-key rotation, local-proxy Host-token rebinding, and runtime-affecting persistence/rebind failures. Repeated requests in the same generation remain terminally rejected; only a later owned `fresh` generation clears the latch. A pure lock-external fetch/auth failure does not poison an otherwise current generation and may be retried manually.
|
||||
- Provider runtime-affecting persistence and message/command/summarize acceptance are linearized by one manager-scoped FIFO coordinator. The coordinator is held only through runtime HTTP acceptance, not the model's reply duration, so it closes credential/config races without reintroducing a global reply lock.
|
||||
- Direct-credential refresh coalescing is scoped by manager identity. A stopped manager cannot lend a no-refresh result to a different running manager.
|
||||
- Runtime registry/config inspection and message, command, and summarize acceptance carry one abort signal with a hard 10-second bound. Timeout releases both manager and project Agent critical sections; the Works model-config fetch and response-body read use the same bounded pattern before entering the manager coordinator.
|
||||
- Timed-out runtime-config leases are revoked. A late non-cooperative continuation cannot mutate readiness or write a second response; an uncertain timeout retains a sticky latch across ordinary fresh restarts until a later successful explicit apply owns a new fresh generation.
|
||||
- Manual runtime start, stop, and restart share the same manager FIFO as provider persistence. A lifecycle action therefore cannot capture partially persisted credentials, and an explicit apply consumes an existing deferred latch even when the persisted provider values already match.
|
||||
- Project Agent observation, live-registry reads, and acceptance share the execution AbortSignal from lock acquisition onward. Cancelling while queued releases that request's FIFO tail, prevents late state observation/runtime calls, and leaves subsequent project mutations unblocked.
|
||||
- Project Agent configuration mutation and Agent registry preflight plus runtime acceptance share one per-project critical section. A concurrent config save cannot complete between a successful live-registry check and `prompt_async` or Agent-scoped command acceptance.
|
||||
- The no-automatic-restart boundary now covers ordinary messages, project commands, and context summarization. Agent-scoped commands use the same atomic readiness barrier as messages.
|
||||
- Model-page and Provider-settings background synchronization also explicitly uses deferred refresh. Only clearly labeled user-triggered apply actions may restart the runtime; successful apply clears any pending UI even when the response reports that a refresh was required.
|
||||
- Added a 10-second, run-scoped prompt startup confirmation window. Host acceptance and a persisted user prompt no longer count as start acknowledgement; explicit busy/retry state, assistant output, question, permission, or a typed terminal event does.
|
||||
- The startup window is enforced by an independent `{sessionId, runToken}` watchdog after Host POST success. It races status polling, message polling, and polling sleeps, so a hung read cannot extend the deadline; late responses and events cannot write after the run token is invalidated.
|
||||
- An unconfirmed start now terminates only the affected session with stable `SESSION_START_UNCONFIRMED`, clears only that session's sending/stream state, and never auto-replays its prompt. Other running sessions remain intact.
|
||||
- Any uncertain remote failure clears that Session's internal queued prompts. A later manual retry sends only the new prompt and cannot replay a prompt queued before the failure; other Sessions' queues and running state are unchanged.
|
||||
- Session run failures are stored only in `sessionRunStates[sessionId]`; the top-level error is reserved for genuine global errors. Chat projection prefers a genuine global error and otherwise uses the selected Session error, without comparing strings for provenance.
|
||||
- Added an Electron E2E regression in which Session A stays busy while Session B receives the typed Agent-registry rejection; B terminates without retry, its draft/error persist, and A's loader/state remain active.
|
||||
- Kept runtime reload/restart/dispose manual. The implementation does not infer whole-instance quiescence from session status.
|
||||
|
||||
## Verification
|
||||
|
||||
- Initial independent Sol review: FAIL. It identified attached-runtime generation provenance, side-effectful retired-file cleanup, prompt-path automatic restart, error provenance collision, and README concurrency wording as blockers. All five findings were remediated with focused regressions; a second independent review is pending.
|
||||
- Second independent Sol review: FAIL. It found a separate legacy credential-import restart path, the config PUT route's remaining side-effectful baseline read, a polling-dependent rather than hard 10-second watchdog, and an overly permissive `starting` provenance. All four findings were remediated with focused regressions; a final re-review is pending.
|
||||
- Third independent Sol review: FAIL. It found a Renderer pre-send provider refresh that could still restart the shared runtime, incomplete generation-latched credential freshness, queued-prompt replay after uncertain startup failure, and a preflight-to-send race between Agent config writes and prompt acceptance. It also identified `/command` and `/summarize` as shared-runtime execution paths requiring the same no-automatic-restart boundary. All findings were remediated with focused regressions; a new read-only re-review is pending.
|
||||
- Fourth independent Sol review: FAIL. It found that runtime-config mutation/latching and final message/command/summarize acceptance were not yet linearized under one manager-scoped coordinator, that two other Renderer background synchronization effects still used automatic `apply`, that direct-credential refresh promises were shared across managers, and that an `apply` partial failure could persist new credentials without retaining a stale-runtime latch. It also identified unbounded runtime HTTP inside the Agent critical section. All findings were remediated with controlled interleaving, cross-manager, partial-failure, and timeout/liveness regressions; a new final read-only review is pending.
|
||||
- Fifth independent Sol review: FAIL. Although its independent 13-file run passed 471/471 tests, it found four remaining coordination gaps: an explicit `apply` after a prior deferred persistence skipped restart because the persisted values already matched; the 10-second `Promise.race` released the manager lease while a non-cooperative late operation could still mutate readiness or write a second HTTP response; manual runtime lifecycle routes did not share the manager FIFO and could start a stale-but-`fresh` generation during delayed persistence; and a non-cooperative `listAgents` call could hold the project lock forever after the manager timeout. Remediation must make explicit apply consume the pending latch, serialize lifecycle with persistence, revoke timed-out leases and suppress all late side effects/responses, and give registry inspection the same abort race as acceptance before another final review.
|
||||
- Fifth-remediation Main focused tests: 4 files, 162/162 passed. They include controlled defer-to-apply, delayed-persistence versus lifecycle, sticky uncertain-latch, original AbortError propagation, late response suppression, non-cooperative registry/config/rebind operations, and abort-while-queued project-lock regressions. Typecheck, scoped lint, `build:vite`, diff check, and project-doc drift also passed after the final signal-checkpoint hardening.
|
||||
- Unified regression set after fifth remediation: 13 files, 487/487 passed.
|
||||
- Full unit suite after fifth remediation: 176 files, 2098/2098 passed.
|
||||
- Full ESLint after fifth remediation passed with zero errors and the same seven pre-existing warnings.
|
||||
- Focused Electron E2E `opencode-multichat-runtime.spec.ts` after final FIFO hardening: 1/1 passed in 6.6 seconds.
|
||||
- The first sixth-review pass was interrupted when main-thread inspection found that an aborted project-lock waiter deleted its queued tail before the active predecessor released, allowing a later mutation to bypass mutual exclusion. A RED A-held/B-abort/C-must-wait interleaving reproduced the bypass; cleanup now retains the cancelled tail until its predecessor chain settles, and the regression passes while also proving later progress.
|
||||
- Sixth independent Sol review: FAIL. Standards passed, but Spec found that a sticky uncertain latch survived a successful explicit provider apply while the runtime was stopped: the route reported success, Renderer cleared its pending UI, and the next fresh start still could not clear the sticky latch, so execution remained terminally rejected. Remediation must convert the sticky latch to clear-on-next-fresh only after the stopped-runtime explicit apply fully persists the account/default selection, and must cover `sticky -> stop -> apply -> start fresh -> execution accepted` before another final review.
|
||||
- Sixth-remediation Main focused tests: 4 files, 164/164 passed. The complete stopped-runtime recovery sequence now ends with a 202 message after the next fresh start without calling restart during apply; a failed default-account persistence remains sticky across a later fresh generation.
|
||||
- Final full unit suite: 176 files, 2100/2100 passed.
|
||||
- Final focused Electron E2E: 1/1 passed in 10.4 seconds.
|
||||
- Seventh independent Sol review: PASS. Standards and Spec both found no blockers. It independently confirmed stopped sticky recovery, timeout/late-continuation safety, single-response protection, manager/project FIFO ordering including A-held/B-abort/C-must-wait, Agent signal propagation, Renderer defer/apply behavior, and the absence of a reply-duration global lock or automatic runtime restart/dispose.
|
||||
- Main remediation tests: 152/152 passed; routes were also rerun independently at 91/91.
|
||||
- Renderer error-provenance tests: 224/224 passed.
|
||||
- Unified focused unit set after second remediation: 9 files, 429/429 passed.
|
||||
- Full unit suite after second remediation: 176 files, 2067/2067 passed.
|
||||
- Third-remediation Main focused tests: 112/112 passed, including controlled Agent config-write versus runtime-acceptance interleaving.
|
||||
- Third-remediation Renderer automatic-sync tests: 98/98 passed; uncertain-failure queue tests: 149/149 passed.
|
||||
- Unified focused regression set after third remediation: 10 files, 446/446 passed.
|
||||
- Full unit suite after third remediation: 176 files, 2074/2074 passed.
|
||||
- Fourth-remediation Main focused tests: 4 files, 145/145 passed; background-sync Renderer tests: 2 files, 7/7 passed.
|
||||
- Unified focused regression set after fourth remediation: 13 files, 471/471 passed.
|
||||
- Full unit suite after fourth remediation: 176 files, 2081/2081 passed.
|
||||
- TypeScript typecheck passed.
|
||||
- Scoped ESLint passed; full lint passed with seven pre-existing warnings in unrelated files.
|
||||
- `build:vite` passed with existing dynamic-import and chunk-size warnings only.
|
||||
- Focused Electron E2E `opencode-multichat-runtime.spec.ts` after fourth remediation: 1/1 passed in 6.9 seconds.
|
||||
- `git diff --check` passed; PowerShell reported line-ending conversion warnings only.
|
||||
- The opt-in real bundled OpenCode smoke was safely skipped because `NIANCODE_OPENCODE_REAL_SMOKE=1` was not configured. No paid provider call was made, so actual provider/runtime two-session execution is not claimed by this task.
|
||||
|
||||
## Follow-ups
|
||||
|
||||
- Run the opt-in real bundled OpenCode two-session smoke with an explicitly configured test provider to establish whether the upstream runtime/provider truly executes two model turns concurrently. The current tests establish application-side isolation, bounded failure, and no replay, not upstream concurrency.
|
||||
- If immediate Agent hot reload is required, first add an upstream directory-scoped authoritative invalidation API or a whole-instance quiescence oracle. Until then, edit/new Agent changes require a manual runtime restart after active replies finish.
|
||||
|
||||
## Promotion Candidates
|
||||
|
||||
- Target: `.project-docs/20-architecture/data-flow.md`. Proposal: document the Main-owned Agent readiness flow (`project.json` write -> desired fingerprint -> runtime generation bootstrap -> live `/agent` id verification -> prompt preflight) and Renderer's per-session startup acknowledgement/terminalization contract. Evidence: focused Main/Renderer unit suites plus `opencode-multichat-runtime.spec.ts`. Expected impact: future prompt, runtime lifecycle, and Agent configuration changes retain the no-replay and no-automatic-dispose safety boundary. No known conflict; human confirmation is not required because this records implemented architecture.
|
||||
Reference in New Issue
Block a user