feat(sessions): 支持事件引用已挂载文件 - #227
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSession events now support Files API references. Public payloads retain file IDs, while validated mounted resources become Sandbox paths in worker payloads. Session creation, event submission, activation, realtime delivery, deployment runs, and resource deletion enforce the new binding rules. ChangesSession event file references
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SessionsService
participant SessionEventWriteTx
participant CodeSession
participant Sandbox
Client->>SessionsService: Submit session event with file_id
SessionsService->>SessionEventWriteTx: Lock session and load active bindings
SessionEventWriteTx-->>SessionsService: Return file ID, path, and MIME type
SessionsService->>SessionEventWriteTx: Validate and append public event
SessionsService->>CodeSession: Queue public event with bindings
CodeSession->>Sandbox: Convert file reference to mounted path
CodeSession-->>Client: Deliver worker event through session delivery
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ca6b91b to
9e291f7
Compare
|
@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: 9e291f705e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
本 PR 在 Code Session 的 activation/realtime 转换阶段重新解析文件引用并硬失败：一旦某条已被 API 接受的事件引用的文件未挂载或已被卸载，activation 会整体回滚导致 run 启动失败。而 deployment 创建的 run 写 initial_events 时完全绕过本 PR 新增的挂载校验，之前可运行的 deployment 可能因此直接失败。建议在 deployment run 路径补上同一套挂载校验，并统一 activation 与 realtime 对无法解析引用的处理策略。
Reviewed changes
本 PR 让 user.message 的 document / image content block 可通过 source.type=file + file_id 引用已挂载的 Files API 文件，并只在 Code Session inbound 边界转换为 Claude Code @"/mnt/session/uploads/..." 路径引用。
- 新增
internal/sessioneventfiles包：校验 file-source block、拒绝客户端本地路径、仅对user.message生效，并在 worker 边界把公开 block 替换为去重后的路径 mention。 - Session 创建新增
initial_events支持；events.send移入持有 Session 行锁的WithSessionEventWriteTx，挂载校验与事件落库同事务原子完成。 internal/codesessions的 realtime 投递与 activation 回放共用workerPayloadForPublicEvent转换；公开边界保留原始file_id，worker payload 存/mnt/session/uploads/...。- DB 新增
ListEventFileBindings查询与CreateSessionInput.InitialEvents，XML 仅按 UUID/workspace 关联、无外键。 - 新增本地 E2E 脚本与 3 个 API 集成测试，覆盖 realtime/activation 转换、整批回滚、创建时 initial_events 引用。
⚠️ Deployment run 的 initial_events 绕过挂载校验，且 activation 对无法解析的引用硬失败
activation 与 realtime 在事件已被 API 接受（200、公开 session_events 已落库）后重新按当前 DB 挂载绑定解析文件引用，两条路径对“绑定消失”的处理不一致：
- realtime（
internal/codesessions/service.go:82-83）：转换失败仅continue，事件被静默丢弃，客户端已收到成功响应但该消息（含文本）永远不会投递给 worker； - activation（
internal/codesessions/managed_agent_code_session.go:171-176）：转换失败直接return err，整个WithManagedAgentActivationTx回滚，后续 cleanup 终止 code session，run 的 agent 不会启动。
而 deployment 创建的 run 完全不经本 PR 新增的 ValidateMountedReferences / ValidatePublicEvent：internal/deployments/handler.go 的 runRoute → sessionEventsFromInitialEvents → CreateManualDeploymentRun（internal/db/deployments.go:211 直接 insertSessionEventsTx），且 deployment 创建时 validateContentSource（internal/deployments/handler.go:1602）允许 source.type=file 但不校验 file_id 是否属于 deployment.resources。因此在 initial_events 引用未挂载文件的 deployment，本 PR 之前会原样透传并成功启动，现在会直接失败（触发链已由独立核查确认）。
同样的硬失败还会出现在：事件已发送后、activation 前卸载了对应 file resource；以及 realtime 路径下发送事务提交到 queue 重新查询之间资源被并发删除（窗口极小）。
Technical details
# 已接受事件中的文件引用在转换期硬失败
## Affected sites
- internal/codesessions/managed_agent_code_session.go:171-176 — activation 转换失败 `return err`，整体回滚，code session 被 cleanup 终止
- internal/codesessions/service.go:80-83 — realtime 转换失败仅 `continue`，事件静默丢弃（API 已返回 200）
- internal/deployments/handler.go:566,632 + internal/db/deployments.go:211 — deployment run 的 initial_events 不经过 ValidateMountedReferences/ValidatePublicEvent 直接落库
- internal/deployments/handler.go:1602-1604 — deployment 创建允许 source.type=file 但不校验 file_id ∈ deployment.resources
## Required outcome
- deployment run 路径与 sessions API 创建路径应用同一套挂载校验；或在 deployment 创建/更新时拒绝引用未挂载文件的 initial_events
- activation 对无法解析的历史文件引用应有明确策略：跳过该引用而不是让整个 replay 失败（与 realtime 的 skip 行为对齐），或保证 send 时已校验的绑定在转换期仍有效
- realtime 路径不要静默丢弃已接受的事件，至少让失败可告警、可追踪
## Open questions for the human
- 是否应阻止卸载被事件引用过的资源？
- activation 失败后是否需要向用户暴露具体错误或提供重试？ℹ️ Nitpicks
- 新增的 3 个 API 测试只覆盖 sessions API 路径；deployment run 路径（initial_events + 文件引用）是本次发现的回归点但没有测试。
internal/sessioneventfiles/references_test.go:89的第一条断言对 “file is not attached” 和 “image mime type mismatch” 两个 case 是恒真 no-op（ValidatePublicEvent对这两类返回nil，err != nil恒为假），读起来像在断言实际未覆盖，建议改为直接比对ValidatePublicEvent的返回。
DeepSeek Flash ｜ 𝕏
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
tests/sessions_api_test.go (1)
3754-3756: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert that initial-event worker payloads exclude
file_id.The current assertion passes if the worker payload contains both the mounted path and the original
file_id. Reject"file_id"here, as Lines 3646-3647 already do for activation replay of submitted events.Proposed test change
- if len(inbound) != 2 || !bytes.Contains(inbound[1].Payload, []byte(`/mnt/session/uploads/initial.pdf`)) { + if len(inbound) != 2 || + !bytes.Contains(inbound[1].Payload, []byte(`/mnt/session/uploads/initial.pdf`)) || + bytes.Contains(inbound[1].Payload, []byte(`"file_id"`)) { t.Fatalf("initial inbound events = %#v, want mounted path", inbound) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/sessions_api_test.go` around lines 3754 - 3756, Update the initial inbound-event assertion in the session test to require the mounted path while also rejecting any `"file_id"` field in the worker payload, matching the existing activation-replay assertion near lines 3646-3647. Keep the event-count check and failure diagnostics intact.internal/sandboxmount/path.go (1)
72-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the shared path check to remove duplication with
FileBackingPath.Lines 75-85 repeat the
filestorepath.Validatecall and the control-character loop fromFileBackingPath(Lines 54-61). Extract one helper and call it from both functions. This keeps the two functions inside the duplicate-code budget.♻️ Proposed refactor
+func validateBackingPath(label, value string) error { + if err := filestorepath.Validate(value, false); err != nil { + return fmt.Errorf("%s %w", label, err) + } + for _, r := range value { + if unicode.IsControl(r) { + return fmt.Errorf("%s must not contain control characters", label) + } + } + return nil +} + func SandboxFilePath(backingPath string) (string, error) { - if err := filestorepath.Validate(backingPath, false); err != nil { - return "", fmt.Errorf("file backing path %w", err) + if err := validateBackingPath("file backing path", backingPath); err != nil { + return "", err } if !strings.HasPrefix(backingPath, FileSource+"/") { return "", fmt.Errorf("file backing path must be under %q", FileSource) } - for _, value := range backingPath { - if unicode.IsControl(value) { - return "", errors.New("file backing path must not contain control characters") - } - } return SandboxUploadsMount + strings.TrimPrefix(backingPath, FileSource), nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sandboxmount/path.go` around lines 72 - 88, Extract the shared backing-path validation currently duplicated in SandboxFilePath and FileBackingPath into a private helper, including filestorepath.Validate and the control-character check. Call that helper from both functions, preserving their existing path-prefix validation and error behavior.Source: Coding guidelines
internal/sessioneventfiles/references.go (1)
63-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReturn a nil payload when validation fails.
WorkerPayloadreturnsrawtogether with a non-nil error at Line 66. A caller that logs the error and still forwards the first return value sends the unexpanded public payload, includingfile_id, to the worker. Every other failure path returnsnil. Make this path consistent.♻️ Proposed change
references, err := referencesFromEvent(eventType, raw) - if err != nil || len(references) == 0 { - return raw, err + if err != nil { + return nil, err + } + if len(references) == 0 { + return raw, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sessioneventfiles/references.go` around lines 63 - 67, Update WorkerPayload so the referencesFromEvent validation-error path returns a nil payload alongside the error, while preserving the existing raw-payload return when no references are found without an error.internal/sessioneventfiles/references_test.go (3)
89-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the
ValidatePublicEventassertion non-vacuous.The condition only fails when the error is non-nil and the message does not match. If
ValidatePublicEventreturnsnil, the check passes. The "client local path" case then proves nothing about the public boundary. Add an expectation for whether each case must fail at the public boundary.💚 Proposed change
tests := []struct { name string payload string bindings []Binding + public bool want string }{raw := json.RawMessage(test.payload) - if err := ValidatePublicEvent("user.message", raw); err != nil && !strings.Contains(err.Error(), test.want) { - t.Fatalf("ValidatePublicEvent() error = %v, want containing %q", err, test.want) + publicErr := ValidatePublicEvent("user.message", raw) + if test.public && (publicErr == nil || !strings.Contains(publicErr.Error(), test.want)) { + t.Fatalf("ValidatePublicEvent() error = %v, want containing %q", publicErr, test.want) + } + if !test.public && publicErr != nil { + t.Fatalf("ValidatePublicEvent() error = %v, want nil", publicErr) }Set
public: truefor the "client local path" case andpublic: falsefor the two mount-dependent cases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sessioneventfiles/references_test.go` around lines 89 - 91, Make the ValidatePublicEvent test assertion non-vacuous by adding the per-case public-boundary expectation: set public to true for “client local path” and false for both mount-dependent cases. Update the test logic to assert whether ValidatePublicEvent("user.message", raw) returns an error according to that expectation, while retaining the existing message-content check for expected failures.
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOrder the failure test before the success tests.
TestReferenceValidationFailuresis declared afterTestWorkerPayloadInjectsDeduplicatedMountedPaths. Move the failure test above both success tests.As per coding guidelines: "Order tests with failure scenarios before success scenarios".
Also applies to: 60-60, 100-100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sessioneventfiles/references_test.go` at line 10, Reorder the test declarations in the references test file so TestReferenceValidationFailures appears before both success tests, including TestWorkerPayloadInjectsDeduplicatedMountedPaths, while leaving the test implementations unchanged.Source: Coding guidelines
20-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd coverage for a file reference in a non-
user.messageevent.
referencesFromEventrejects file blocks wheneventType != "user.message"(Lines 140-142 ofinternal/sessioneventfiles/references.go). No test exercises that branch. Add a case that passessystem.messagewith asource.type=fileblock.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sessioneventfiles/references_test.go` around lines 20 - 58, Extend the references test coverage to call WorkerPayload with event type system.message and a payload containing a source.type=file block, then assert referencesFromEvent rejects or handles it according to the existing non-user-message behavior. Keep the current user.message assertions unchanged and ensure the new case exercises the eventType != "user.message" branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/be/session-startup-message-delivery.md`:
- Around line 11-14: Update the design statement around session_events.payload
and Files API references to document both conversion boundaries: activation
replay and active realtime delivery via workerPayloadForPublicEvent. Clarify
that conversion produces Claude Code mount paths for inbound events while the
original session_events.payload remains unchanged.
In `@internal/codesessions/service.go`:
- Line 80: Update the event conversion flow around workerPayloadForPublicEvent
so any conversion error is returned through the existing retryable delivery path
before QueueRawPublicSessionEvents, rather than logged and skipped. Preserve
successful handling of valid events, and add a mixed-batch test covering one
valid event and one unresolved file reference to verify persistence, retry, and
delivery behavior.
In `@internal/db/session_resource_mapper.xml`:
- Around line 137-143: Update the ListEventFileBindings query to require an
active mount by adding the resource.expires_at predicate alongside the existing
resource filters. Extend the relevant mapper assertion in
sessions_mapper_test.go to verify this expiry condition is present.
In `@tests/sessions_api_test.go`:
- Around line 3559-3709: Reorder the test functions so
TestSessionEventMissingFileResourceRejectsWholeBatch appears before the
successful TestSessionEventFileReferencesUseMountedResources scenario. Do not
change either test’s implementation or assertions.
---
Nitpick comments:
In `@internal/sandboxmount/path.go`:
- Around line 72-88: Extract the shared backing-path validation currently
duplicated in SandboxFilePath and FileBackingPath into a private helper,
including filestorepath.Validate and the control-character check. Call that
helper from both functions, preserving their existing path-prefix validation and
error behavior.
In `@internal/sessioneventfiles/references_test.go`:
- Around line 89-91: Make the ValidatePublicEvent test assertion non-vacuous by
adding the per-case public-boundary expectation: set public to true for “client
local path” and false for both mount-dependent cases. Update the test logic to
assert whether ValidatePublicEvent("user.message", raw) returns an error
according to that expectation, while retaining the existing message-content
check for expected failures.
- Line 10: Reorder the test declarations in the references test file so
TestReferenceValidationFailures appears before both success tests, including
TestWorkerPayloadInjectsDeduplicatedMountedPaths, while leaving the test
implementations unchanged.
- Around line 20-58: Extend the references test coverage to call WorkerPayload
with event type system.message and a payload containing a source.type=file
block, then assert referencesFromEvent rejects or handles it according to the
existing non-user-message behavior. Keep the current user.message assertions
unchanged and ensure the new case exercises the eventType != "user.message"
branch.
In `@internal/sessioneventfiles/references.go`:
- Around line 63-67: Update WorkerPayload so the referencesFromEvent
validation-error path returns a nil payload alongside the error, while
preserving the existing raw-payload return when no references are found without
an error.
In `@tests/sessions_api_test.go`:
- Around line 3754-3756: Update the initial inbound-event assertion in the
session test to require the mounted path while also rejecting any `"file_id"`
field in the worker payload, matching the existing activation-replay assertion
near lines 3646-3647. Keep the event-count check and failure diagnostics intact.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d7f0993d-14f8-4b58-86a0-a0135258d7fd
📒 Files selected for processing (21)
docs/design/be/session-event-file-references.mddocs/design/be/session-startup-message-delivery.mdinternal/codesessions/managed_agent_code_session.gointernal/codesessions/mapper.gointernal/codesessions/service.gointernal/db/managed_agent_activation.gointernal/db/session_event_writes.gointernal/db/session_resource_mapper.gointernal/db/session_resource_mapper.xmlinternal/db/sessions.gointernal/db/sessions_helpers.gointernal/db/sessions_mapper_test.gointernal/sandboxmount/path.gointernal/sessioneventfiles/references.gointernal/sessioneventfiles/references_test.gointernal/sessions/event_files.gointernal/sessions/file_resources.gointernal/sessions/service.gointernal/sessions/service_helpers.goscripts/test-session-file-events-local.shtests/sessions_api_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/deployments/handler.go`:
- Around line 765-777: Replace the map-based decoding in the deployment
resource-loading flow with a typed resource-reference DTO. Validate the DTO’s
type, file_id, and memory_store_id fields during unmarshalling or immediately
afterward, rejecting malformed or unsupported references instead of silently
converting failed assertions to empty strings. Update the resource loop and its
file/memory-store handling to consume the validated DTO fields.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 68188f75-ddce-4921-bb17-5d4c7b299cc1
📒 Files selected for processing (18)
docs/design/be/session-event-file-references.mddocs/design/be/session-startup-message-delivery.mdinternal/codesessions/service.gointernal/db/session_event_mapper.gointernal/db/session_event_mapper.xmlinternal/db/session_event_writes.gointernal/db/session_resource_mapper.xmlinternal/db/sessions.gointernal/db/sessions_mapper_test.gointernal/deployments/handler.gointernal/deployments/resources.gointernal/sandboxmount/path.gointernal/sessioneventfiles/references.gointernal/sessioneventfiles/references_test.gointernal/sessions/event_files.gointernal/sessions/service.gotests/deployments_api_test.gotests/sessions_api_test.go
💤 Files with no reviewable changes (1)
- internal/db/session_event_writes.go
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/sandboxmount/path.go
- internal/sessions/event_files.go
- internal/db/session_resource_mapper.xml
- internal/sessions/service.go
- internal/sessioneventfiles/references_test.go
- internal/sessioneventfiles/references.go
- docs/design/be/session-event-file-references.md
There was a problem hiding this comment.
ℹ️ 本轮增量修复扎实，上一轮 review 的核心风险已被完整覆盖。仅发现一处与新增资源 pin 不变量相关的小边界不一致，见行内注释。
Reviewed changes
本轮对上一轮 pullfrog review（9e291f7）的反馈做了系统性修复，并补齐了对应测试：
- Deployment run 路径在创建 Session 前，用本次运行物化出的 File 绑定校验
initial_events，未挂载/未出现在deployment.resources的引用以session_resource_not_found_error失败且不创建 Session（含 image MIME 校验）。 - Realtime
QueuePublicSessionEvents改为使用 Send 事务内已校验的绑定快照，转换失败由静默continue改为返回错误并记录结构化日志。 - 新增
HasFileReferenceForResourcepin：被公开事件引用的 File Resource 删除返回 409（ErrFileInUse），与 Send 写入共用同一 Session 行锁，消除“已接受事件随后资源被删除”的竞态。 ListEventFileBindings增加expires_at活动性过滤；WorkerPayload校验失败返回 nil payload。- 测试修复：
TestReferenceValidationFailures增加publicError期望并补充system.message文件块用例（原先对两个挂载依赖 case 是恒真断言），失败场景测试前移；新增整批回滚、session_thread_id拒绝、资源删除 409、Deployment run 成功/失败路径等测试。
设计文档同步更新了 realtime 绑定快照、资源 pin 与 Deployment run 校验的语义，且与实现一致。
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
ℹ️ d637632 改动干净，未引入新问题：把 Deployment run 校验里的松散
map[string]any断言替换为带类型校验的解析器，并补齐了契约测试。
Reviewed changes
本轮增量（d637632 fix(deployments): type stored resource references）与上一轮审查内容独立，只覆盖 deployment 存储资源引用的解析健壮性：
- 新增
parseDeploymentRunResourceReferences：用类型化deploymentRunResourceReference结构解析deployment.Resources，按file/memory_store/github_repository分别校验对应引用字段，拒绝未知类型与跨类型字段污染。 validateRunReferences改用该解析器，替代原先resource["file_id"].(string)这类会静默吞掉错误字段类型的断言。TestParseDeploymentRunResourceReferences覆盖 9 个拒绝用例与 1 个正常用例，失败场景先行，测试可证伪。
解析器与 sessionResourcesFromDeployment（materialize 路径）在同一份存储 JSON 上保持一致，github_repository 在 validateRunReferences 中仍为无操作项，行为与旧实现对齐。未发现新问题。
上一轮针对 HasFileReferenceForResource 的 file_id 空白填充不对称发现仍然存在，且与本提交无关，保持开放。
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
✅ 本轮增量（
ebbd5bdmerge +ff2a5ec）改动干净，未引入新问题。上一轮针对资源 pin 的空白不对称发现已在ff2a5ec从两侧闭合，新增测试可证伪。
Reviewed changes
本轮相对上一轮 pullfrog review（d637632）的增量是 ff2a5ec 及一次 main 合并：
- 修复资源 pin 的空白不对称：
referencesFromEvent现在拒绝带首尾空白的非规范file_id（internal/sessioneventfiles/references.go:150），HasFileReferenceForResource的 pin 查询改用BTRIM(...)做防御性匹配（internal/db/session_event_mapper.xml:152），并同步更新了 Mapper SQL 断言与设计文档校验清单。 - 补齐可证伪测试：
TestReferenceValidationFailures新增 "file ID has surrounding whitespace" 用例（publicError: true），失败路径先于成功用例。 - 适配 main 的严格 DTO 解码（#229 风格）：
sessionMutationRequest增加InitialEvents字段，create用body.InitialEvents取代fields["initial_events"]，resourceFromRequest直接读取body.FileID/body.URL，与主分支DecodeObjectBodyAs[T]接线一致。 - 本地验证：
go build ./...、go vet、受影响 package（sessioneventfiles/sandboxmount/deployments/sessions/codesessions/db）单元测试全部通过。
上一轮唯一的行内发现（pin SQL 空白不对称）已由本提交修复，相关 thread 已被作者标记解决；本 repo 的 Pullfrog 线程已全部关闭。
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/deployments/resources.go (1)
463-484: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject duplicate file mounts with the same
FileExternalID.
resolveReferenceskeeps the first binding for eachFileID, so duplicate mounts resolve to the first path without an error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/deployments/resources.go` around lines 463 - 484, Update deploymentEventFileBindings to track each resource.FileMount.FileExternalID as it is processed and return an error when the same ID appears more than once. Perform this duplicate check before appending the binding, while preserving the existing missing-file validation and binding behavior for unique mounts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/deployments/resources.go`:
- Around line 361-378: Update the stored resource validation loop to reject
surrounding whitespace in both FileID and MemoryStoreID, matching the
canonical-ID checks used by the event-reference validator; require each present
ID to equal its trimmed value while preserving the existing type and
mutual-exclusion rules, and add table cases covering whitespace-padded IDs.
---
Outside diff comments:
In `@internal/deployments/resources.go`:
- Around line 463-484: Update deploymentEventFileBindings to track each
resource.FileMount.FileExternalID as it is processed and return an error when
the same ID appears more than once. Perform this duplicate check before
appending the binding, while preserving the existing missing-file validation and
binding behavior for unique mounts.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e9fa42d6-dc81-4114-bf90-24bc54b3df84
📒 Files selected for processing (11)
docs/design/be/session-event-file-references.mdinternal/db/session_event_mapper.xmlinternal/db/sessions_mapper_test.gointernal/deployments/handler.gointernal/deployments/handler_contract_test.gointernal/deployments/resources.gointernal/sessioneventfiles/references.gointernal/sessioneventfiles/references_test.gointernal/sessions/handler.gointernal/sessions/service.gointernal/sessions/service_helpers.go
🚧 Files skipped from review as they are similar to previous changes (8)
- internal/sessioneventfiles/references_test.go
- internal/db/sessions_mapper_test.go
- internal/sessions/service_helpers.go
- internal/db/session_event_mapper.xml
- internal/sessioneventfiles/references.go
- docs/design/be/session-event-file-references.md
- internal/deployments/handler.go
- internal/sessions/service.go
There was a problem hiding this comment.
ℹ️ 本轮增量删除本地真实服务 E2E 脚本
scripts/test-session-file-events-local.sh（459 行），未引入新逻辑，也未提供替代脚本；但设计文档「验收」一节与 PR 描述仍引用该脚本，存在失效引用，见行内注释。
Reviewed changes
- 删除本地真实服务 E2E 脚本
scripts/test-session-file-events-local.sh，无替代实现；tests/e2e/下现存的 Files E2E（Go/Python/TS）均不覆盖“会话文件事件引用”链路。 - 设计文档
session-event-file-references.md的「验收」一节仍指导运行该脚本，未同步更新。
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
ℹ️ 本轮增量把上一轮遗留的失效引用清掉了文档一半，改动干净。PR 描述中"增加真实本地服务 E2E 脚本"一句仍未更正，相关 thread 保持部分开放。
Reviewed changes
本轮增量（1056946）只改 docs/design/be/session-event-file-references.md：
- 删除「验收」一节中指导运行已删除的
scripts/test-session-file-events-local.sh的整段说明（14 行），文档与仓库现状一致；全仓库 grep 已无该脚本引用。 - 上一轮 thread（
session-event-file-references.md:153）的文档一半已闭合，但 PR 描述「变更摘要」仍声称"增加真实本地服务 E2E 脚本"、「测试」节仍称"真实本地 E2E 通过"，与删除后的仓库状态不符——这一半由 open thread 继续跟踪，待作者决定恢复脚本或更正描述。
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found. 本轮增量把存储资源引用校验与事件引用校验的空白规范对齐，改动干净，测试可证伪。
Reviewed changes
本轮增量（b7f64f6）相对上一轮 pullfrog review（1056946）只改了 deployment 存储资源引用解析的空白校验：
parseDeploymentRunResourceReferences拒绝带首尾空白的 ID：file_id和memory_store_id现在要求TrimSpace(v) == v，与referencesFromEvent（ff2a5ec）的 canonical-ID 校验对齐，消除"存储引用接受空白填充但事件引用校验拒绝"的不一致。- 新增两个可证伪测试用例：
"file ID has surrounding whitespace"和"memory store ID has surrounding whitespace"加入拒绝列表，若解析器接受空白填充 ID 则测试会失败。
所有先前 Pullfrog 线程均已在 GitHub 上 [RESOLVED]。
openai-compatible/glm-5.2 ｜ 𝕏
…ile-references # Conflicts: # internal/deployments/handler.go # internal/sessions/service.go
There was a problem hiding this comment.
✅ No new issues found. 本轮增量只是把最新
main（#232 错误适配器标准化）合入分支，冲突解决干净，PR 原有文件引用不变量全部保留。
Reviewed changes
本轮相对上一轮 pullfrog review（b7f64f6）的增量是 cfee78c merge，引入 main 的 #232 Standardize resource error adapters，并手写解决了 internal/deployments/handler.go 与 internal/sessions/service.go 的冲突：
- Deployment
runRoute适配 error-returning 模式：validateRunReferences仍返回(filesByID, referenceError)；deploymentEventFileBindings与逐条ValidateMountedReferences校验保留；writeRunReferenceFailure改为返回error（内部写 200 + failed run JSON 后返回 nil，不会与 ErrorAdapter 双写）。 - Session
sendEventsRoute/create/deleteResourceRoute适配：Send 事务内的绑定快照 +QueuePublicSessionEvents(..., eventBindings)接线不变；原先deleteResourceRoute内联的ErrFileInUse→409 逻辑移入mapResourceLoadError（internal/sessions/errors.go:125-127），线缆合同一致（409conflict_error）。 internal/sessions/errors.go新增ErrFileInUse映射，与HasFileReferenceForResourcepin 的既有测试契约吻合。
本地验证：go build ./... 与受影响 package（sessions/deployments/sessioneventfiles/sandboxmount/codesessions/httpapi）单元测试全部通过。设计文档无需更新（数据边界、pin 与校验语义均未变化）。
DeepSeek Flash ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
本轮相对上一轮 pullfrog review（cfee78c，4904493219）的增量是 ed16231 fix(sessions): align resource ID matching：
- 把资源 ID 匹配契约从"在校验边界拒绝带空白填充的 file_id"（ff2a5ec/b7f64f6 的 canonical 形式）统一为"原文保留 + 精确匹配"：
referencesFromEvent只拒绝缺失或 trim 后为空的file_id，带空白填充的值被保留，随后在挂载绑定精确匹配处落空并返回 400 "is not mounted"（internal/sessioneventfiles/references.go:146-150）。 HasFileReferenceForResourcepin 查询去掉防御性BTRIM，改为content_block->'source'->>'file_id' = file.external_id精确比对（internal/db/session_event_mapper.xml:152）。由于所有落库路径（send、创建 initial_events、deployment run）都先经过精确匹配的挂载校验，session_events.payload中不可能存在带填充的 file_id，精确比对与存储数据一致，BTRIM 不再必要。parseDeploymentRunResourceReferences保留带空白填充的存储引用（仅拒绝 trim 后为空），填充值随后在GetFile的external_id精确查询处使 run 失败，与事件侧行为一致（已核实GetFileSQL 为精确匹配）。- 设计文档校验清单第 4 条与 Mapper SQL 断言同步更新为精确匹配契约；测试全部可证伪：all-whitespace 拒绝、padded file_id 精确匹配落空、"preserves resource IDs with surrounding whitespace" 契约用例、padded file_id 400 API 集成测试。
本地验证：go build ./... 通过；sessioneventfiles、deployments、db、sessions、codesessions、sandboxmount 单元测试全部通过。所有先前 Pullfrog 线程均已 [RESOLVED]，PR 描述也已更正（不再声称存在本地 E2E 脚本）。
接受集合与上一版本完全相同（带空白填充的 ID 在两个版本下都被拒绝，仅错误文案从 "must not contain surrounding whitespace" 变为 "is not mounted"）；这是设计文档明确记载的有意取舍，三个校验面（事件引用、资源 pin、Deployment 存储引用）现在共用同一条精确匹配规则。
DeepSeek Flash ｜ 𝕏
|
There was a problem hiding this comment.
ℹ️ 本轮增量（
44d6763）把 worker 内容准备移入 Session 写事务并细化输入/内部错误分类，改动扎实、测试可证伪。仅发现一处与重构相关的失效文档注释，见文末 Nitpicks。
Reviewed changes
本轮相对上一轮 pullfrog review（b514451）的增量是 44d6763 fix(sessions): tighten event file processing：
- Worker 内容准备移入写事务：
events.send在WithSessionEventWriteTx内用事务绑定快照为每条事件调用prepareEventWorkerContent，全部准备成功才插入公开 payload；QueuePublicSessionEvents不再重新解析file_id/挂载路径，只按event.ExternalID取出已准备内容并补充 Code Session envelope（workerPayloadFromPublicEvent），消除了"公开事件已提交但 worker 内容未准备好"的中间状态——任一输入或转换失败整批回滚。 - 输入/内部错误分类：
sessioneventfiles新增validationError/IsValidationError，sessions新增eventInputError/eventProcessingError；mapEventProcessingError把客户端输入错误映射为 400，把存量状态解析、挂载路径、序列化等内部错误映射为 500，同时应用于events.send与创建initial_events。 - Deployment run 资源规划重构：
validateRunReferences返回解析后的[]deploymentRunResource（保留 raw JSON），sessionResourcesFromDeployment与deploymentEventFileBindings合并为planDeploymentSessionResources（一次解析，返回 resources + eventBindings 计划），行为与旧实现等价（secret 下标、filesByID键、类型处理均保持一致）。 - 测试：新增
TestMapEventProcessingErrorDistinguishesInputAndInternalFailures、TestNormalizeInputEventClassifiesInputAndInternalErrors、API 级TestSessionEventInternalNormalizationFailureReturnsServerError（500 + 整批不落库）、TestPlanDeploymentSessionResourcesSharesFileBinding、TestPlanSessionResourceWritesSharesFileBinding；TestReferenceValidationFailures补充断言IsValidationError。
本地验证：go build ./...、go vet、受影响 package（codesessions/sessions/deployments/sessioneventfiles/sandboxmount/sessioncontract）单元测试全部通过。realtime 与 activation 的转换结果保持一致（prepared payload 与重新转换输出同一 envelope），未发现行为回归。
ℹ️ Nitpicks
internal/deployments/resources.go:85与:160两处文档注释仍引用已删除的sessionResourcesFromDeployment，本提交已将其重命名为planDeploymentSessionResources，建议同步更新注释，避免误导后续维护者。
DeepSeek Flash ｜ 𝕏
1 participant
变更摘要
user.message，支持在document/imagecontent block 中通过source.type=file + file_id引用已挂载的 Files API 文件。initial_events与运行中的events.send使用同一套文件引用校验，并要求文件已通过 Session Resources API 挂载。file_id；只在 Code Session inbound 边界转换为 Claude Code@"/mnt/session/uploads/..."路径引用。设计与数据边界
不增加新的 Session Event payload 字段或 migration。事件列表、SSE、Webhook、审计和前端展示都不会读取或泄漏 worker 绝对路径。详细设计见
docs/design/be/session-event-file-references.md。测试
main的受影响 package 测试全部通过：internal/sessioneventfiles、internal/sessions、internal/codesessions、internal/db、internal/sandboxmount。initial_events文件引用。兼容说明
main，不依赖 feat(agents): 支持自定义 MCP 服务器配置 #218。Summary by CodeRabbit