Conversation
🦋 Changeset detectedLatest commit: c9eadb9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80b8b92498
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const model = boundModel ?? (this.config.get<string>('defaultModel') || null); | ||
| const modelSource = boundModel !== undefined ? 'agent' : model !== null ? 'default' : 'unknown'; | ||
| event.handle.accessor.get(ITelemetryService).withContext({ model }).track2('session_started', { |
There was a problem hiding this comment.
For direct SDKRpcClientV2.createSession({ model }) calls, the SDK invokes ISessionManager.create without a mainAgentBinding and only materializes/binds input.model afterward (packages/node-sdk/src/sdk-rpc-client-v2.ts:1363-1379). This snapshot therefore reports defaultModel (or unknown) rather than the requested model; the SDK forwarding appender flattens that context into the event properties, so direct SDK telemetry is now incorrectly attributed whenever the requested model differs from the default. Pass the initial binding through or defer the startup event until after the SDK applies the model.
AGENTS.md reference: AGENTS.md:L98-L100
Useful? React with 👍 / 👎.
| const model = body.agent_config?.model || undefined; | ||
| const handle = await core.accessor.get(ISessionManager).create({ | ||
| workspaceId: touched.id, | ||
| workDir, | ||
| mainAgentBinding: model === undefined ? undefined : { profile: DEFAULT_AGENT_PROFILE_NAME, model }, |
There was a problem hiding this comment.
When a client creates a session for an unregistered metadata.cwd with an unknown model, createOrTouch has already persisted and published the workspace before this binding reaches model validation. Session creation cleans up its session directory on that failure but does not remove the workspace, so the request returns an error while GET /workspaces subsequently exposes a workspace the user never successfully created. Validate the model before touching the workspace, or undo a newly created workspace on this failure path.
AGENTS.md reference: AGENTS.md:L98-L100
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60a328c96e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const sessionId = input.id ?? `session_${randomUUID()}`; | ||
| const registration = manager.onDidCreateSession((event) => { | ||
| if (event.sessionId !== sessionId) return; | ||
| event.waitUntil((async () => { |
There was a problem hiding this comment.
When a direct SDK caller supplies an unknown model (or any model/thinking/permission combination that makes materializeMainAgent reject), this listener puts the failure into waitUntil; AsyncEmitter only reports rejected waiters and still lets manager.create resolve. Before this change the same await ran after creation and rejected createSession, so callers now receive a successful, persisted session whose requested configuration was silently not applied. Propagate this promise's failure out of doCreateSession (while retaining the startup-event ordering) instead.
AGENTS.md reference: AGENTS.md:L118-L120
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Requirement or Bug
Desktop 创建或恢复会话时的
session_started缺少模型归因，创建请求显式指定的模型也会被忽略。Bug Reproduction Steps
example-model，通过POST /api/v1/sessions传入agent_config.model: "mock-model"。session_started记录。Root Cause
创建处理器没有把请求中的模型传给会话初始化，直接 SDK 调用也在引擎启动事件之后才绑定模型。模型只写入 Agent 的埋点上下文，而启动事件从 Session 层发出。上报器也没有区分未指定模型与明确未知的模型。本修改打通模型绑定与事件归因，不改变事件的生命周期触发时机。
Code Changes
同步中英文服务端 API 文档，并增加真实创建/恢复路径与发送载荷的回归测试。
Behavior Changes and Affected Users
property_model_sourcecontext_model，其他事件的兜底保持原样未指定模型时不新增强制绑定；创建、恢复、重复打开活跃会话的事件计数保持原有语义。恢复使用已有主 Agent 绑定；后续切换模型不会改写启动快照。Desktop/Web 的现有客户端已经发送该创建参数，未增加请求字段或磁盘格式。Desktop 需要在后续内嵌核心版本更新中包含本提交。
验证：
context_model和单事件 null 清除。pnpm lint通过，无错误；保留仓库现有警告。pnpm --dir docs run build通过，中英文说明保持同步。git diff --check通过。Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.