From fbfcd69c2bb17087535ea4af679bc2bfb9838897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BF=AE=E9=9B=A8?= <47820304+PeterGuy326@users.noreply.github.com> Date: Thu, 25 Jun 2026 10:43:21 +0800 Subject: [PATCH] feat(connect): make /new vs /clear use each channel's real session op Before, /new and /clear were identical: both just dropped the local conversationId->sessionId mapping. The agent-side session was never disposed (opencode sessions leaked), and the two commands had no real difference beyond their ack text. Align them to each channel's real capability: - /new (resetSession): drop the local mapping only; the old agent session is left intact and stays resumable where the agent supports it. - /clear (new sessionClearer): actively dispose the current session via the agent's real delete primitive. opencode implements it with DELETE /session/:id (idempotent on 404). Channels whose agent exposes no delete in the mode DWS drives it (Codex app-server, Qoder stream, Claude exec) fall back to resetSession, so /clear behaves like /new there. Tests pin both directions: /clear issues exactly one DELETE and drops the mapping; /new (resetSession) issues zero DELETEs so the old session stays resumable. Doc updated to describe the per-channel behavior. go build ./... ok; go vet ok; go test ./internal/helpers ok. --- internal/helpers/connect_opencode.go | 39 +++++++- internal/helpers/connect_opencode_test.go | 98 +++++++++++++++++++ internal/helpers/connect_stream.go | 34 ++++++- .../multi/dingtalk-dev/references/connect.md | 3 +- 4 files changed, 167 insertions(+), 7 deletions(-) diff --git a/internal/helpers/connect_opencode.go b/internal/helpers/connect_opencode.go index 9c2f603c..a66932fc 100644 --- a/internal/helpers/connect_opencode.go +++ b/internal/helpers/connect_opencode.go @@ -159,14 +159,34 @@ func (f *opencodeForwarder) close() error { } // resetSession drops the conversation's opencode session so the next message -// starts a fresh one. Implements sessionResetter for the /new and /clear -// commands. A no-op when per-conversation memory is disabled. +// starts a fresh one, leaving the old session on the opencode server (it stays +// resumable by id). Backs the /new command. A no-op when memory is disabled. func (f *opencodeForwarder) resetSession(convID string) { if f.sessions != nil { f.sessions.reset(convID) } } +// clearSession backs the /clear command: it actively deletes the conversation's +// session on the opencode server (DELETE /session/:id) and then drops the local +// mapping, so the context is truly wiped rather than just forgotten. A missing +// server session is treated as already cleared. Implements sessionClearer. +func (f *opencodeForwarder) clearSession(ctx context.Context, convID string) error { + if f.sessions == nil { + return nil + } + sessionID := f.sessions.id(convID) + f.sessions.reset(convID) + if strings.TrimSpace(sessionID) == "" { + return nil + } + client, err := f.server.ensure(ctx) + if err != nil { + return err + } + return client.deleteSession(ctx, sessionID) +} + type opencodeServer struct { bin string env []string @@ -332,6 +352,21 @@ func (c *opencodeHTTPClient) createSession(ctx context.Context) (string, error) return out.ID, nil } +// deleteSession disposes a session on the opencode server (DELETE /session/:id). +// A 404 (session already gone) is treated as success so /clear stays idempotent. +func (c *opencodeHTTPClient) deleteSession(ctx context.Context, sessionID string) error { + if strings.TrimSpace(sessionID) == "" { + return nil + } + if err := c.doJSON(ctx, http.MethodDelete, "/session/"+sessionID, nil, nil); err != nil { + if errors.Is(err, errOpencodeSessionMissing) { + return nil + } + return err + } + return nil +} + func (c *opencodeHTTPClient) sendMessage(ctx context.Context, sessionID, text, model string) (string, error) { body := map[string]any{ "parts": []map[string]any{{"type": "text", "text": text}}, diff --git a/internal/helpers/connect_opencode_test.go b/internal/helpers/connect_opencode_test.go index bcbfbf13..9e0e6147 100644 --- a/internal/helpers/connect_opencode_test.go +++ b/internal/helpers/connect_opencode_test.go @@ -313,3 +313,101 @@ func TestOpencodeForwarderMessageGovernedByTurnCtx(t *testing.T) { t.Fatal("a reply slower than the turn ctx should fail") } } + +// opencodeForwarder must satisfy sessionClearer so /clear gets a real delete. +var _ sessionClearer = (*opencodeForwarder)(nil) + +// TestOpencodeForwarderClearDeletesServerSession pins /clear semantics: it issues +// a real DELETE /session/:id on the opencode server and drops the local mapping. +func TestOpencodeForwarderClearDeletesServerSession(t *testing.T) { + dir := t.TempDir() + var deleted []string + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.URL.Path == "/global/health": + _, _ = w.Write([]byte(`{"healthy":true}`)) + case r.Method == http.MethodPost && r.URL.Path == "/session": + _, _ = w.Write([]byte(`{"id":"ses_clear"}`)) + case r.Method == http.MethodPost && r.URL.Path == "/session/ses_clear/message": + _, _ = w.Write([]byte(`{"parts":[{"type":"text","text":"hi"}]}`)) + case r.Method == http.MethodDelete && r.URL.Path == "/session/ses_clear": + deleted = append(deleted, r.URL.Path) + _, _ = w.Write([]byte(`{}`)) + default: + http.NotFound(w, r) + } + })) + defer ts.Close() + + f := &opencodeForwarder{ + bin: "opencode", + timeout: 5 * time.Second, + workDir: dir, + sessions: newOpencodeSessions(filepath.Join(dir, "s.json")), + server: &opencodeServer{baseURL: ts.URL, httpClient: ts.Client()}, + } + + if _, err := f.forwardStream(context.Background(), "conv-1", "hi", nil); err != nil { + t.Fatalf("forward: %v", err) + } + if got := f.sessions.id("conv-1"); got != "ses_clear" { + t.Fatalf("session = %q, want ses_clear", got) + } + + if err := f.clearSession(context.Background(), "conv-1"); err != nil { + t.Fatalf("clearSession: %v", err) + } + if len(deleted) != 1 || deleted[0] != "/session/ses_clear" { + t.Fatalf("server deletes = %v, want one DELETE /session/ses_clear", deleted) + } + if got := f.sessions.id("conv-1"); got != "" { + t.Fatalf("session after /clear = %q, want empty", got) + } +} + +// TestOpencodeForwarderResetKeepsServerSession pins /new semantics: resetSession +// only drops the local mapping and must NOT delete the server-side session, so +// the old session stays resumable (the /new vs /clear distinction). +func TestOpencodeForwarderResetKeepsServerSession(t *testing.T) { + dir := t.TempDir() + deletes := 0 + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.URL.Path == "/global/health": + _, _ = w.Write([]byte(`{"healthy":true}`)) + case r.Method == http.MethodPost && r.URL.Path == "/session": + _, _ = w.Write([]byte(`{"id":"ses_keep"}`)) + case r.Method == http.MethodPost && r.URL.Path == "/session/ses_keep/message": + _, _ = w.Write([]byte(`{"parts":[{"type":"text","text":"hi"}]}`)) + case r.Method == http.MethodDelete: + deletes++ + _, _ = w.Write([]byte(`{}`)) + default: + http.NotFound(w, r) + } + })) + defer ts.Close() + + f := &opencodeForwarder{ + bin: "opencode", + timeout: 5 * time.Second, + workDir: dir, + sessions: newOpencodeSessions(filepath.Join(dir, "s.json")), + server: &opencodeServer{baseURL: ts.URL, httpClient: ts.Client()}, + } + + if _, err := f.forwardStream(context.Background(), "conv-1", "hi", nil); err != nil { + t.Fatalf("forward: %v", err) + } + f.resetSession("conv-1") + if deletes != 0 { + t.Fatalf("resetSession issued %d DELETE(s), want 0 (old session must stay resumable)", deletes) + } + if got := f.sessions.id("conv-1"); got != "" { + t.Fatalf("session after /new = %q, want empty", got) + } +} diff --git a/internal/helpers/connect_stream.go b/internal/helpers/connect_stream.go index 04bdfc4b..c212121b 100644 --- a/internal/helpers/connect_stream.go +++ b/internal/helpers/connect_stream.go @@ -82,6 +82,16 @@ type sessionResetter interface { resetSession(convID string) } +// sessionClearer is an optional capability beyond sessionResetter: a forwarder +// that can actively dispose a conversation's session on the agent side (e.g. +// opencode's DELETE /session/:id), so /clear truly wipes it instead of only +// forgetting the local id mapping. Channels whose agent exposes no delete in +// the mode DWS drives it fall back to sessionResetter, so /clear behaves like +// /new there. +type sessionClearer interface { + clearSession(ctx context.Context, convID string) error +} + type forwarderCloser interface { close() error } @@ -1004,10 +1014,26 @@ func runStreamConnector(ctx context.Context, channel, clientID, clientSecret str if action, isCmd := parseConnectControlCommand(text); isCmd { ackText := action.ack if action.resetsSession() { - if r, canReset := fwd.(sessionResetter); canReset { - r.resetSession(convID) - } else { - ackText = "当前渠道暂不支持会话指令（/new、/clear）。" + cleared := false + // /clear prefers a real server-side session delete when the + // channel's agent supports one (opencode). /new always just + // drops the local mapping so the old session stays resumable. + if action.name == "clear" { + if c, canClear := fwd.(sessionClearer); canClear { + cctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + if err := c.clearSession(cctx, convID); err != nil { + fmt.Fprintf(os.Stderr, "[connect] /clear 删除会话失败 (%s, msgId=%s): %v\n", channel, msgID, err) + } + cancel() + cleared = true + } + } + if !cleared { + if r, canReset := fwd.(sessionResetter); canReset { + r.resetSession(convID) + } else { + ackText = "当前渠道暂不支持会话指令（/new、/clear）。" + } } } if serr := replier.SimpleReplyText(context.Background(), webhook, []byte(ackText)); serr != nil { diff --git a/skills/multi/dingtalk-dev/references/connect.md b/skills/multi/dingtalk-dev/references/connect.md index e4cd772e..5a1dbbcb 100644 --- a/skills/multi/dingtalk-dev/references/connect.md +++ b/skills/multi/dingtalk-dev/references/connect.md @@ -80,7 +80,8 @@ dws dev connect --unified-app-id --channel codex --format json ## 机制与环境覆盖 - **stream-bridge 渠道**：Go 原生进程内 Stream 转发器，订阅 `TOPIC_ROBOT`。Claude/CodeBuddy/WorkBuddy/custom 每条 @机器人消息起一个无头 CLI 实例 → stdout 回钉钉；Qoder/QoderWork 会在 connect 生命周期内复用一个常驻 `qodercli --print --output-format stream-json --input-format stream-json` 子进程。 -- **会话记忆**：Codex 记录 `conversationId -> threadId`；opencode 记录 `conversationId -> sessionId` 并通过本地 `opencode serve` 的 HTTP API 续聊；Qoder/QoderWork 记录 `conversationId -> Qoder session_id` 并在常驻 stream-json 子进程里续聊；Claude/CodeBuddy/WorkBuddy 使用 `--session-id`/`--resume`。映射按机器人落盘，重启后可继续；`/new`、`/start`、`/reset`、`/clear` 会清空当前会话映射。 +- **会话记忆**：Codex 记录 `conversationId -> threadId`；opencode 记录 `conversationId -> sessionId` 并通过本地 `opencode serve` 的 HTTP API 续聊；Qoder/QoderWork 记录 `conversationId -> Qoder session_id` 并在常驻 stream-json 子进程里续聊；Claude/CodeBuddy/WorkBuddy 使用 `--session-id`/`--resume`。映射按机器人落盘，重启后可继续。 +- **会话指令 `/new` vs `/clear`**（对齐各渠道真实能力）：`/new`（含 `/start`、`/reset`）开新会话——丢掉当前映射、下一条消息起新 session，**旧 session 保留**（opencode/Codex/Claude 侧仍可按 id 续）；`/clear` 则**真正清掉当前会话**——能删的渠道调 agent 原生删除原语（opencode `DELETE /session/:id`），不暴露删除接口的渠道（Codex/Qoder/Claude 系）降级为与 `/new` 相同的重置。两者都只回 ack、不消耗 agent turn。 - **官方渠道**（openclaw/hermes）：dws 不代建机器人，输出官方 onboarding 指引。 - 环境覆盖：`DWS_AGENT_CMD`(整条命令覆盖,覆盖时不再注入模型/会话参数) / `DWS_AGENT_MODEL` / `DWS_AGENT_WORKDIR` / `DWS_CONNECT_CMD` / `DWS_CONNECT_NO_INSTALL=1` / `DWS_AGENT_TIMEOUT_MS`。