diff --git a/README.md b/README.md index 53d4ad7..691db9c 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,21 @@ Contributions are welcome! See [CONTRIBUTING.md](CONTRIBUTING.md) for setup, code style, commit conventions, and the PR checklist. Notable changes are recorded in [CHANGELOG.md](CHANGELOG.md). +## Related Projects + +- [zcode-open-bridge](https://github.com/tizerluo/zcode-open-bridge) — a community Python implementation that bridges ZCode to the MCP/ACP ecosystem. The design of this server references its bridge architecture and several handling strategies. + +## Acknowledgements + +- [Agent Client Protocol](https://agentclientprotocol.com/) (Apache-2.0) — the ACP specification +- [ZCode](https://zcode.z.ai) / [Zhipu Z.AI](https://z.ai) — the GLM model and ZCode CLI +- [zcode-open-bridge](https://github.com/tizerluo/zcode-open-bridge) — reference implementation that informed this server's design + ## License Apache-2.0. This project follows the same license as the upstream ACP specification. + +## Disclaimer + +This is an independent community project and is not affiliated with, endorsed +by, or sponsored by Zhipu Z.AI. ZCode is a product of Zhipu Z.AI. diff --git a/README.zh-CN.md b/README.zh-CN.md index 9cee31f..39378ff 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -115,6 +115,20 @@ CI 会在每次 push 和 pull request 时运行 `typecheck`、`lint`、`build` 欢迎贡献！请阅读 [CONTRIBUTING.md](CONTRIBUTING.md) 了解环境搭建、代码风格、 commit 约定和 PR 检查清单。重要变更记录在 [CHANGELOG.md](CHANGELOG.md)。 +## 相关项目 + +- [zcode-open-bridge](https://github.com/tizerluo/zcode-open-bridge) —— 一个社区 Python 实现，将 ZCode 接入 MCP/ACP 生态。本项目参考了它的桥接架构和若干处理策略。 + +## 致谢 + +- [Agent Client Protocol](https://agentclientprotocol.com/)（Apache 2.0）—— ACP 协议规范 +- [ZCode](https://zcode.z.ai) / [智谱 Z.AI](https://z.ai) —— GLM 模型与 ZCode CLI +- [zcode-open-bridge](https://github.com/tizerluo/zcode-open-bridge) —— 参考实现，本项目的设计借鉴了它的桥接架构 + ## 许可证 Apache-2.0。本项目沿用上游 ACP 规范的同一许可证。 + +## 免责声明 + +本项目为独立的社区项目，与智谱 Z.AI 官方无任何隶属、认可或赞助关系。ZCode 是智谱 Z.AI 的产品。 diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..23b112c --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,40 @@ +# Security Policy + +## What This Project Touches + +This is a stdio ACP bridge — no network listener, no own auth. However, in +order to bridge the headless ZCode CLI to ACP-compatible editors, it does +read and write a small number of sensitive files owned by the ZCode app: + +| Path | Read/Write | What it touches | +|------|-----------|-----------------| +| `~/.zcode/v2/config.json` | read | Reads the active provider's `baseURL` and `apiKey` (in `backend/credentials.ts`) and the model list (in `config/options.ts`, `config/runtime-model.ts`). The `apiKey` is forwarded to the ZCode subprocess via an environment variable (`ANTHROPIC_API_KEY`) and is never written to logs, stdout, or any other file. | +| `~/.zcode/v2/tasks-index.sqlite` | **read/write** | Inserts/updates rows in the `tasks` table (`tasks-index.ts`) so that sessions created via ACP appear in the ZCode app's UI. Only the `tasks` table is touched, using `INSERT OR IGNORE` / bounded `UPDATE`. | + +The bridge does **not**: +- send credentials, tokens, or session data anywhere except the local ZCode subprocess; +- modify the ZCode CLI binary, the app, or any file outside `tasks-index.sqlite`; +- expose any network port. + +## Reporting a Vulnerability + +If you find a way this bridge leaks credentials, corrupts the tasks-index, or +escapes its stdio boundary, please report it privately rather than opening a +public issue: + +- Open a private security advisory via GitHub's + [Report a vulnerability](https://github.com/william0wang/zcode-acp/security/advisories/new), + or +- email the maintainer (see the GitHub profile). + +Please include the affected file/line, a description of impact, and a +reproduction if possible. You should hear back within **72 hours**. + +## Out of Scope + +Report these to the upstreams, not here: + +- The ZCode CLI itself, the ZCode app, or the `config.json` format — these + are owned by [Zhipu Z.AI](https://z.ai). +- The editor (Zed, JetBrains) or the ACP specification. +- Issues that require already having code execution on the host. diff --git a/src/backend/client.ts b/src/backend/client.ts index 3c7beff..1ec3c8b 100644 --- a/src/backend/client.ts +++ b/src/backend/client.ts @@ -53,6 +53,9 @@ export class ZcodeBackend { private readonly serverRequests: ServerRequest[] = []; private readonly listeners = new Map(); private readerDead = false; + /** Monotonic id for fire-and-forget sends (send()). Uses a high range to + * avoid collisions with the server's request ids (low range). */ + private sendIdCounter = 1_000_000_000; /** Watchdog process that kills the zcode group if this bridge dies (SIGKILL). */ private watchdog: ChildProcess | null = null; @@ -213,7 +216,17 @@ export class ZcodeBackend { log("backend: sendReply dropped (stdin closed)"); return; } - stdin.write(JSON.stringify({ id, result }) + "\n"); + try { + stdin.write(JSON.stringify({ id, result }) + "\n"); + } catch (e) { + // Broken pipe: backend is gone. Mark reader dead so the rest of the + // bridge stops trying to talk to it; otherwise zcode reannounces would + // keep hitting a dead pipe and the dedup cache would spin forever. + this.readerDead = true; + log( + `backend: sendReply write failed (marked dead): ${e instanceof Error ? e.message : String(e)}`, + ); + } } /** Reply to a zcode server→client request with an error. */ @@ -223,7 +236,14 @@ export class ZcodeBackend { log("backend: sendError dropped (stdin closed)"); return; } - stdin.write(JSON.stringify({ id, error: { code, message } }) + "\n"); + try { + stdin.write(JSON.stringify({ id, error: { code, message } }) + "\n"); + } catch (e) { + this.readerDead = true; + log( + `backend: sendError write failed (marked dead): ${e instanceof Error ? e.message : String(e)}`, + ); + } } // ---------- send / request ---------- @@ -238,6 +258,23 @@ export class ZcodeBackend { stdin.write(JSON.stringify({ method, params }) + "\n"); } + /** + * Send a message with an id but WITHOUT registering a pending response + * (fire-and-forget). Mirrors Python's `_backend.send({"id": ..., ...})` for + * `session/stop`: some backends route by id presence, so carrying an id is + * more robust than a bare notify. If the backend replies, the reader's + * `resolvePending` finds no pending entry and safely discards it. + */ + send(method: string, params?: Record): void { + const stdin = this.proc.stdin; + if (!stdin || stdin.destroyed) { + log("backend: send dropped (stdin closed)"); + return; + } + const id = this.sendIdCounter++; + stdin.write(JSON.stringify({ id, method, params: params ?? {} }) + "\n"); + } + /** * Synchronous request/response: register a pending promise, send, await. * Other notifications arriving during the wait are routed async by the @@ -304,9 +341,13 @@ export class ZcodeBackend { } // Wait up to 3s for a clean exit. const exited = await new Promise((resolve) => { - const done = () => resolve(true); + let timer: ReturnType; + const done = () => { + clearTimeout(timer); // don't let the timeout keep the event loop alive + resolve(true); + }; proc.once("exit", done); - setTimeout(() => { + timer = setTimeout(() => { proc.removeListener("exit", done); resolve(false); }, 3000); diff --git a/src/handlers/session.ts b/src/handlers/session.ts index 78b6c21..c9508cf 100644 --- a/src/handlers/session.ts +++ b/src/handlers/session.ts @@ -281,19 +281,20 @@ export async function prompt( const intercepted = await handleSlashCommand(server, cx, params.sessionId, zcodeSid, text); if (intercepted) return intercepted; - // Preempt: if another turn is still running for this session (client sent a - // new prompt without cancelling), stop it and wait for it to fully exit - // before we subscribe/send. Without this, session/send hits the backend - // prompt-lock and the error path kills the old turn but loses the new msg. - await preemptInFlightTurn(server, zcodeSid, requestId); - - // Register the pending turn. This same object is mutated by cancel(); the - // turn loop checks `.cancelled` on the SAME reference, so cancel propagates. + // Register self + preempt others under a per-session lock. The lock + // serializes the critical section so that two concurrent prompts (B, C) for + // the same session can't both miss each other and register at once: C waits + // for B's section, by which point B is in pendingTurns, so C's preempt finds + // and cancels B. Registering INSIDE the lock is what makes the new turn + // visible to the next prompt's preempt scan. const turn: PendingTurn = { zcodeSid, cancelled: false, }; - server.pendingTurns.set(requestId, turn); + await withPreemptLock(server, zcodeSid, () => { + server.pendingTurns.set(requestId, turn); + return preemptInFlightTurn(server, zcodeSid, requestId); + }); const listener = new EventStreamListener(backend, zcodeSid); const monitor = new TurnMonitor(backend, zcodeSid, () => server.nextId()); @@ -321,9 +322,9 @@ export async function prompt( 15000, ); if (sendResp.error) { - // send failed/timeout — but backend may have started the turn anyway. - // Stop it and probe to avoid leaking the prompt lock. - await ensureTurnStopped(server, zcodeSid); + // send failed/timeout. Don't fire stop here: a send failure usually + // means the turn never started (no lock to leak). Mirrors Python which + // just returns the error without stopping. throw new Error(`zcode send failed: ${sendResp.error.message ?? ""}`); } const accepted = (sendResp.result ?? {}) as { accepted?: boolean }; @@ -408,75 +409,67 @@ export async function cancel( } /** - * Send `session/stop` and probe until the prompt lock is confirmed released. + * Fire-and-forget `session/stop` to the backend. Mirrors Python's + * `_cancel_backend_turn`: send stop with an id (some backends route by id + * presence), never wait for a response, never throw. * - * `session/stop` is fire-and-forget, but ZCode has a startup delay: when stop - * arrives before the turn truly holds the lock, the backend ignores it and the - * turn runs on, leaking the lock — the next `session/send` then fails with - * "A prompt is already running". Mirrors the `expectLock:true` strategy from - * `waitForTurnIdle` (extensions.ts) but adapted for the cancel path. + * The backend's turn loop will emit turn.completed(cancelled) on its own; + * the ACP turn loop observes that event and exits. No probing needed on the + * prompt path — an earlier ensureTurnStopped probed session/goal show for 30s + * but returned inconsistent values and caused severe stalls. + */ +function stopBackendTurn(server: ZcodeAcpServer, zcodeSid: string): void { + try { + server.ensureBackend().send("session/stop", { sessionId: zcodeSid }); + } catch (e) { + log( + ` [stop] session/stop send failed (ignored): ${e instanceof Error ? e.message : String(e)}`, + ); + } +} + +/** + * Serialize a per-session critical section. Each section awaits the previous + * one's promise before running, so concurrent prompts for the same session + * execute register+preempt strictly one after another. * - * Strategy: - * 1. send `session/stop` - * 2. poll `session/goal show`; first REQUIRE seeing "prompt is running" once - * (proves the turn started), then wait for it to clear - * 3. if the grace window (8s) elapses without ever seeing the lock, the turn - * never started (stop caught it in time) or already ended → treat as released - * 4. hard timeout 30s + * Used by prompt() to wrap "register self in pendingTurns + preempt others": + * the registration must land before the section releases, so the next prompt + * entering its section sees this turn in its preempt scan. Without this lock, + * two near-simultaneous prompts could both scan before either registers. * - * Best-effort: never throws (failures only log) so it can't break the cancel - * path. Returns true if released, false on timeout. + * The body may be async and long-running (preempt waits up to 35s for the old + * turn to exit); that is acceptable because the turn loop itself runs OUTSIDE + * this lock — only registration + preempt-in-wait are serialized. */ -async function ensureTurnStopped(server: ZcodeAcpServer, zcodeSid: string): Promise { - const backend = server.ensureBackend(); - backend.notify("session/stop", { sessionId: zcodeSid }); - const t0 = Date.now(); - const GRACE_MS = 8_000; - const HARD_TIMEOUT_MS = 30_000; - let lockSeen = false; - while (Date.now() - t0 < HARD_TIMEOUT_MS) { - const resp = await backend.request( - server.nextId(), - "session/goal", - { sessionId: zcodeSid, action: "show" }, - 10000, - ); - const errMsg = resp.error?.message ?? ""; - if (errMsg.includes("prompt is running")) { - lockSeen = true; - await sleep(2000); - continue; - } - if (errMsg.toLowerCase().includes("timeout")) { - await sleep(2000); - continue; - } - // Non-lock error or success. - if (lockSeen) { - log(` [stop] prompt lock released`); - return true; - } - // Haven't seen the lock yet — give the turn a grace window to start. - if (Date.now() - t0 < GRACE_MS) { - await sleep(500); - continue; +function withPreemptLock( + server: ZcodeAcpServer, + zcodeSid: string, + body: () => Promise, +): Promise { + const prev = server.preemptLocks.get(zcodeSid) ?? Promise.resolve(); + const next = prev.then(body, body); // run body regardless of prior rejection + server.preemptLocks.set(zcodeSid, next); + // Clean up the entry once settled so a later idle session doesn't retain a + // dangling promise. Only delete if still ours (a newer section may have + // chained on top of us). + next.finally(() => { + if (server.preemptLocks.get(zcodeSid) === next) { + server.preemptLocks.delete(zcodeSid); } - // Grace window expired without ever seeing the lock: turn never started - // (stop caught it in time) or already ended. Safe to treat as released. - log(` [stop] no lock observed within grace window, treating as released`); - return true; - } - log(` [stop] lock wait timed out (30s), lock may still be held`); - return false; + }); + return next; } /** - * If another prompt() is already running for this zcodeSid, treat the new - * prompt as an implicit cancel: stop the in-flight turn and wait for it to - * fully exit (lock released + listener unregistered + pendingTurns cleaned) - * before the new prompt subscribes and sends. + * Cancel any other in-flight turn for this zcodeSid and wait for it to fully + * exit (listener unregistered + pendingTurns cleaned) before returning. + * + * Must be called from inside a preempt lock section (the caller has already + * registered itself in pendingTurns), so a concurrent prompt entering its own + * section is guaranteed to see this caller's turn and cancel it. * - * Why wait for the map entry to disappear (not just the lock): registering + * Why wait for the map entry to disappear (not just fire stop): registering * a second EventStreamListener overwrites the first (Map.set in client.ts), * so the old turn loop must have run its finally block before we subscribe. * The map cleanup in that finally block is the synchronization point. @@ -501,13 +494,15 @@ async function preemptInFlightTurn( if (oldRequestId === undefined) return; // no in-flight turn, proceed log(` [preempt] in-flight turn ${oldRequestId} found, stopping it`); - // Stop the backend turn and wait for the prompt lock to release. - await ensureTurnStopped(server, zcodeSid); + // Fire-and-forget stop (mirrors Python's _cancel_backend_turn). The old + // turn loop will receive turn.completed(cancelled) and exit on its own. + stopBackendTurn(server, zcodeSid); // Wait for the old turn's prompt() to fully exit (its finally block deletes // the pendingTurns entry). This is the synchronization point that guarantees - // its listener is unregistered before we register ours. - const PREEMPT_TIMEOUT_MS = 35_000; // slightly longer than ensureTurnStopped's 30s + // both lock release (backend turn ended) and listener unregistration before + // we subscribe/send. More reliable than probing session/goal show. + const PREEMPT_TIMEOUT_MS = 35_000; const t0 = Date.now(); while (server.pendingTurns.has(oldRequestId)) { if (Date.now() - t0 > PREEMPT_TIMEOUT_MS) { @@ -604,7 +599,7 @@ async function runEventTurn( } if (turn.cancelled) { - await ensureTurnStopped(server, turn.zcodeSid); + stopBackendTurn(server, turn.zcodeSid); return { stopReason: "cancelled" }; } @@ -626,7 +621,7 @@ async function runEventTurn( await sendTextChunk(cx, acpSid, reply, chunkMsgId); } else if (!emittedOutput) { // No text and no output → suspected failure. - await ensureTurnStopped(server, turn.zcodeSid); + stopBackendTurn(server, turn.zcodeSid); throw new RequestError(-32603, "turn produced no output"); } } @@ -686,13 +681,14 @@ async function runEventTurn( } if (translator.turnDone) { - // Cancel signalled via turn.completed(resultType:"cancelled"). + // Cancel signalled via turn.completed(resultType:"cancelled"). The + // backend turn has already ended and released the lock — no stop needed. if (translator.turnResultType === "cancelled") { - await ensureTurnStopped(server, turn.zcodeSid); return { stopReason: "cancelled" }; } if (translator.turnFailed) { - await ensureTurnStopped(server, turn.zcodeSid); + // Best-effort stop in case the failed turn left a residual lock. + stopBackendTurn(server, turn.zcodeSid); throw new RequestError(-32603, formatTurnError(translator.turnError)); } // Fallback: if no text streamed, surface the last assistant reply. @@ -728,7 +724,7 @@ async function runEventTurn( } // 120s no progress: abandon. - await ensureTurnStopped(server, turn.zcodeSid); + stopBackendTurn(server, turn.zcodeSid); return { stopReason: "max_turn_requests" }; } diff --git a/src/server.ts b/src/server.ts index e0c114e..fa50f28 100644 --- a/src/server.ts +++ b/src/server.ts @@ -39,6 +39,12 @@ export class ZcodeAcpServer { readonly sessionMap = new Map(); /** Currently running turns, keyed by the ACP request id. */ readonly pendingTurns = new Map(); + /** + * Per-session (zcodeSid) preempt lock: a promise chain that serializes the + * "register self + preempt others" critical section in prompt(). Prevents + * concurrent prompts from both missing each other and registering at once. + */ + readonly preemptLocks = new Map>(); /** Capabilities advertised by the connected client (Zed, JetBrains, ...). */ clientCapabilities: ClientCapabilities = {}; /** Session titles already set, to enforce set-once (acp_sid → title). */ diff --git a/src/translators/event-translator.ts b/src/translators/event-translator.ts index 02ac2ed..36da98b 100644 --- a/src/translators/event-translator.ts +++ b/src/translators/event-translator.ts @@ -203,11 +203,11 @@ export class EventTranslator { private translateTurnDone(payload: Record): InternalEvent[] { const usage = (payload["usage"] as Record) ?? {}; - const used = (usage["totalTokens"] as number) ?? (payload["tokenCount"] as number) ?? undefined; - const size = (usage["contextWindow"] as number) ?? 0; - if (typeof used === "number") { - return [{ kind: "UsageDelta", used, size }]; - } - return []; + // Use || (not ??) to match Python's `or` semantics: a falsy totalTokens + // (0 / undefined) falls back to tokenCount, then to 0. With ?? a 0 would + // be kept as-is and never fall back, diverging from the Python reference. + const used = (usage["totalTokens"] as number) || (payload["tokenCount"] as number) || 0; + const size = (usage["contextWindow"] as number) || 0; + return [{ kind: "UsageDelta", used, size }]; } }