fix(cli): cancel MCP management operations

[liuxiaocs7]

Summary

Cancel MCP management work when the user leaves an in-flight TUI action with Esc or q. Cancellation now propagates through sync, test, reconnect, disconnect, OAuth/provider credential updates, and remote publication; stdio children are reaped, safely reversible late writes are rolled back, and revision/CAS fences prevent cancelled or superseded results from replacing newer state.

The operation budget is bounded at 90 seconds with 5 seconds reserved for cleanup, and the TUI reports cancellation and rollback failures in the supported English, Simplified Chinese, and Traditional Chinese copy catalogs.

Fixes #4846
Refs #3838

Verification

• npm run lint
• npm run format:check
• npm run build
• npm run typecheck
• npx knip --workspace apps/desktop
• npx knip --workspace packages/ui
• MCP test suite: 185 passed
• CLI test suite: 823 passed, 3 skipped (826 total)
• npm run check:tui-copy

Regression tests reproduce cancellation during pending MCP transport, stdio lifecycle, config publication, host-capability publication, and credential writes, and verify cleanup/rollback plus stale-result suppression.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope:

OpenAI Codex — diagnosis, implementation, regression tests, verification, and review support. The affected commit includes the required Generated-by: OpenAI Codex trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Reviewed current head a83668152758423eb3133244a95a765698d30722 (OPEN). Two P2s below, both reproduced against real stores. (Note: the reviewing lane operates under a no-GitHub instruction from its owner, so this synthesis is published by the orchestrating lane instead.)

P2 — read-before-CAS without a revision cannot stop ABA

host-profile.ts:404-423 / credential-store.ts:170-187: rollback CAS compares only the credential string or null — no revision. Reproduced against a real file store: store old-value; a cancelling delete does old -> null; another instance then set(newer-value) and explicitly deletes to null; the old operation's CAS(null, old-value) still returns true, resurrecting an already-deleted old credential. A second path (cancelled-value -> newer-value -> cancelled-value) lets a stale rollback overwrite a newer write. A cancelled operation can thus undo a newer administrative action. Fix: carry a monotonic revision/opaque generation on records, return that token from reads, and compare on token rather than secret text. Existing tests (host-profile.test.ts:563-582, credential-store.test.ts:315-353) only cover "new value differs", never ABA.

P2 — delete reports success even when the CAS missed

host-profile.ts:1487-1498: deleteCapabilityProviderCredential discards committed: false on a post-read CAS failure yet still resolves successfully. Deterministic repro: a concurrent write of same owner/incarnation newer-value between getSecret and CAS yields deleteResolved: true while the final credential is still newer-value. Callers cannot distinguish "deleted" from "raced by an update", and will commit wrong local state as success. Fix: return whether the delete committed, or take the previously-read revision/basis from the caller and propagate the conflict. New tests do not cover delete CAS-miss.

What I could not judge

Power-loss / directory-fsync fault injection was not run.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

本条结论全部来自 @Haoqing_Reviewer 的审查。我自己没有读这份 diff；我核的是当前 head 有没有漂移。当前 head 是 a836681，未关闭。两条 P2 都是凭据 CAS 竞态：回滚无版本号挡不住 ABA 致旧凭据复活；删除失败仍报成功致误判。修好再合。

[Astro-Han]

Follow-up to my review above, same head a83668152758423eb3133244a95a765698d30722: two more P2s from the same lane, both reproduced on this head.

P2 — cancellation starts transport.close() fire-and-forget; overlapping server instances possible

packages/mcp/src/index.ts:2657-2661: cancellation starts transport.close() without awaiting it, so the operation can reject before a stdio child is actually reaped. openClient() then calls safeClose(), but SDK v2 clears its process handle at the start of the first close(), so the second close cannot await the in-progress shutdown. Reproduced on this head: a stdio child ignoring stdin EOF and SIGTERM made sync(..., { signal }) reject after 35 ms while its PID was still alive; an immediate retry spawned a second child 25 ms later with both PIDs alive, the first reaped ~4.0 s later by SIGKILL only. This permits overlapping server instances after a reported cancellation — port/file contention and continued side effects. Fix: keep the abort-triggered close promise and await it before connectCandidate settles. The new test (manager.test.ts:1295-1338) only polls for exit after sync already rejected, so it does not enforce this ownership boundary; use a shutdown-resisting child and assert the PID is gone before rejection settles.

P2 — clearAuthorization drops the abort signal on reconnect and swallows the error

packages/mcp/src/index.ts:1697-1704: clearAuthorization(..., { signal }) forwards the signal to credential deletion but drops it when calling reconnect(serverId), then swallows the reconnect error. Reproduced on this head: with a remote endpoint switched to hold requests, clearAuthorization with remoteConnectMs: 300 aborted after 20 ms stayed in connecting and fulfilled with an error status after 306 ms instead of rejecting (up to ~30 s with defaults). Fix: pass the options into reconnect and preserve abort rejection instead of swallowing it; add a regression for cancellation after credential deletion but during reconnect.

Checks: @maka/mcp typecheck green, manager.test.js 66/66 on Node 22.22.1, git diff --check clean. No files or GitHub state were modified by the review.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

本条是上一条的补充，同样来自 @Haoqing_Reviewer 的审查。另两条 P2：取消时不等子进程回收完就返回，可致服务端实例重叠；清理授权时丢掉取消信号并吞掉重连错误。修好再合。

[Astro-Han]

Second follow-up to my reviews above, same head a83668152758423eb3133244a95a765698d30722: two more P2s from the CLI slice, both reproduced on this head. Six P2s total across the three comments, no P0/P1/P3.

P2 — editing an endpoint irreversibly clears OAuth credentials, then reports cancellation

tui-mcp-control.ts:580-591: editing/deleting an endpoint first irreversibly clears the old OAuth credential; if Esc/q aborts after that point, the call returns cancelled while keeping the old configuration. Reproduced: old URL with OAuth token → edit to new URL → press Esc after the store deletion starts → result status=failed, reason=cancelled, config still the old URL, credential gone, snapshot still reports configuration ready. Reconnecting the old endpoint now requires re-authentication. The existing test (tui-mcp-control.test.ts:985-1037) only asserts the old config is kept, never that the old credential survives — so it passes a destructive "cancel" as success. Either complete the commit once irreversible deletion has started, or make credential retirement transactionally recoverable.

P2 — rotation CAS-miss leaves the connection pointing nowhere

tui-mcp-remote-publication.ts:255-263: when rotating credentials, a CAS-miss on concurrent update keeps the newer credential but never re-reads and connects it; the old connection was already closed at :250, so publication parks at host_unavailable indefinitely. Reproduced: A connected; rotate to B (write B, disconnect A, B connect pending); another writer writes C; press Esc; B→A CAS fails; final store is C but only A and B were ever connection-attempted — C never connected, availability host_unavailable, old connection closed. The existing test (tui-mcp-remote-publication.test.ts:183-217) only checks C was not overwritten, never connection convergence. On CAS-miss, re-read and connect the current credential, or surface an explicit needs-resync (non-cancelled) state.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

本条是第二条补充，同样来自 @Haoqing_Reviewer 的审查。再加两条 P2：编辑端点先不可逆清凭据再报取消，致旧端点需重认证；轮换 CAS 未命中不断开也不重连，停在不可用。修好再合。