fix(sessions): 启动期用户消息可靠投递 - #188
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
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:
📝 WalkthroughWalkthroughManaged-agent code sessions now start as ChangesSession startup delivery
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SessionAPI
participant session_event_queue
participant EnvironmentRunner
participant CodeSession
participant InboundQueue
Client->>SessionAPI: Send startup user.message
SessionAPI->>session_event_queue: Persist event UUID
EnvironmentRunner->>CodeSession: Create initializing code session
CodeSession->>session_event_queue: Load FIFO queued messages
CodeSession->>InboundQueue: Append history and queued messages
CodeSession->>session_event_queue: Delete delivered queue rows
CodeSession->>CodeSession: Transition initializing to active
Possibly related PRs
Suggested reviewers: 🚥 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 |
3f1aaeb to
3a1a564
Compare
jh0904
changed the title
71aabfb to
39a7942
Compare
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
@codex review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![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: ff51cb6432
ℹ️ 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".
Close the #189 startup window where accepted user messages were only written to session_events and never reached code_session_inbound_events. Record startup delivery responsibility in session_event_queue in the same transaction as session_events, then atomically hand off the full queue, clear it, and activate the Code Session under the same Session row lock used by Send Events. Reject extra ordinary startup user messages with 409.
ff51cb6 to
d0ba785
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0ba785c67
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/codesessions/managed_agent_code_session.go`:
- Around line 135-171: Bound the retry loop surrounding
ActivateManagedAgentCodeSessionWithQueue with a finite attempt limit and return
a clear error when the limit is exhausted. Add a small backoff before retrying
when activated is false, while preserving immediate returns for successful
activation and existing errors; honor context cancellation during the backoff.
In `@internal/db/session_event_queue.go`:
- Around line 212-264: Update sessionUserMessageStartupWindowSQLX in
internal/db/session_event_queue.go:212-264 to derive startup eligibility solely
from the code_sessions lifecycle status, treating initializing sessions as
startup and excluding environment_work.state from the decision. Preserve
existing error handling and session/work matching behavior. The related sites in
internal/db/deployments.go:285-293 and internal/sessions/service.go:599-634
require no direct changes; they document the lifecycle behavior this query must
follow.
🪄 Autofix (Beta)
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: e923c6ed-f898-4e67-ac47-19d7a3dde314
📒 Files selected for processing (17)
docs/design/be/session-startup-message-delivery.mdinternal/codesessions/managed_agent_code_session.gointernal/codesessions/service.gointernal/db/code_sessions.gointernal/db/db.gointernal/db/deployments.gointernal/db/migrations/00036_add_session_event_queue.sqlinternal/db/session_event_queue.gointernal/db/sessions.gointernal/db/sessions_migration_sqlx_test.gointernal/db/sessions_sqlx.gointernal/environments/runner.gointernal/sessions/service.gointernal/sessions/service_helpers.gotests/deployments_api_test.gotests/environments_runner_cloud_test.gotests/sessions_api_test.go
💤 Files with no reviewable changes (1)
- internal/environments/runner.go
Drop overlapping startup-window cases (self-hosted, terminated CS, stopped work, cross-session queue ownership) and fold post-activation realtime assertions into the atomic activation test. Slim the #189 runner and deployment multi-message checks without losing the unique paths.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2151cf7288
ℹ️ 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".
Use samber/lo for small event filters, align the conflict sentinel with other DB errors, and keep activation checks to type match plus queue snapshot comparison.
Align the startup window with Runner managed Code Session activation so self_hosted sessions keep accepting events without an undrained queue.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/environments_runner_cloud_test.go (1)
353-357: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAssert durable queue ownership before code-session creation.
The final inbound assertion proves eventual delivery, but not that the accepted event was persisted in
session_event_queueduring the no-Code-Session window—the regression this test is meant to prevent. Assert the sole queued ID immediately aftersendSessionEvents.Proposed test strengthening
acceptedEventID = sessionEventStringField(t, sent.Data[0], "id") + if queued := sessionEventQueueEventIDs(t, app, session.ID); len(queued) != 1 || queued[0] != acceptedEventID { + t.Fatalf("startup queue = %#v, want [%s]", queued, acceptedEventID) + } },🤖 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/environments_runner_cloud_test.go` around lines 353 - 357, In the test flow around sendSessionEvents, immediately assert that the sole accepted event ID is present in session_event_queue before creating the Code Session. Reuse the existing acceptedEventID and queue-inspection helpers or established database assertion pattern, while preserving the current len(sent.Data) validation and eventual delivery checks.
🤖 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.
Outside diff comments:
In `@tests/environments_runner_cloud_test.go`:
- Around line 353-357: In the test flow around sendSessionEvents, immediately
assert that the sole accepted event ID is present in session_event_queue before
creating the Code Session. Reuse the existing acceptedEventID and
queue-inspection helpers or established database assertion pattern, while
preserving the current len(sent.Data) validation and eventual delivery checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e245588-391f-42f6-9e57-a9021ec5288d
📒 Files selected for processing (8)
docs/design/be/session-startup-message-delivery.mdinternal/codesessions/managed_agent_code_session.gointernal/db/code_sessions.gointernal/db/db.gointernal/db/session_event_queue.gotests/deployments_api_test.gotests/environments_runner_cloud_test.gotests/sessions_api_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/codesessions/managed_agent_code_session.go
- docs/design/be/session-startup-message-delivery.md
Amp-Thread-ID: https://ampcode.com/threads/T-019fb1cb-d867-7480-91fa-b9d947642e90 Co-authored-by: Amp <amp@ampcode.com>
Merge historical session_events with the startup queue on activation so new sandboxes keep prior turns while still delivering startup-window messages reliably.
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)
tests/sessions_api_test.go (1)
3841-3878: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake queue ownership mismatches fail instead of disappearing.
This query does not require
e.session_id = s.id; a queue row can therefore pair the requested session with another session’s event in the same tenant. Because the joins are inner joins, dangling or mismatched rows can also be silently omitted, weakening the tests for the documented ownership and rollback guarantees.Use a left join with explicit session binding so malformed rows cause the helper to fail rather than appear as an empty queue.
Proposed fix
- join session_events e + left join session_events e on e.uuid = q.session_event_uuid + and e.session_id = s.id + and e.session_external_id = s.external_id and e.organization_id = o.id and e.workspace_id = w.id🤖 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 3841 - 3878, Update sessionEventQueueEventIDs to left-join session_events with an explicit e.session_id = s.id binding, while preserving the tenant and workspace ownership predicates. Make the helper detect a missing or mismatched event from the nullable left-join result and fail the test instead of silently omitting the queue row.
🧹 Nitpick comments (2)
tests/sessions_api_test.go (1)
688-1043: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReorder the new Go tests to put failure cases first.
The additions place activation success/realtime coverage before rollback, conflict, and rejected-batch scenarios. Reorder the new cases so failure scenarios precede success scenarios.
As per coding guidelines,
**/*_test.go: 测试组织顺序先写失败场景，再写成功场景。🤖 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 688 - 1043, Reorder the newly added test functions so failure scenarios come before success scenarios: place TestSessionEventQueueDeliveryRollsBackOnInboundFailure, TestSessionStartupRejectsSecondUserMessage, TestSessionStartupRejectedBatchHasNoSideEffects, and TestSessionStartupSerializesConcurrentUserMessages before TestManagedAgentActivationAtomicallyDeliversSessionEventQueue. Do not change test implementations or behavior.Source: Coding guidelines
internal/db/code_sessions.go (1)
359-372: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueQueue-delete statement exists twice. Both sites changed to the same organizations/workspaces UUID-join predicate in this PR, so a future change to queue tenant scoping must be applied in two places and can silently diverge.
internal/db/code_sessions.go#L359-L372: replace the inline literal with the shareddeleteSessionEventQueueQueryconstant.internal/db/sessions_sqlx.go#L127-L136: keep this as the single definition of the queue-delete predicate.🤖 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/db/code_sessions.go` around lines 359 - 372, The queue-delete SQL is duplicated across both call sites. In internal/db/code_sessions.go lines 359-372, replace the inline statement in the deletedResult operation with the shared deleteSessionEventQueueQuery constant; retain internal/db/sessions_sqlx.go lines 127-136 as the sole definition of that predicate.Source: Coding guidelines
🤖 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/codesessions/managed_agent_code_session.go`:
- Around line 189-210: Update listSessionEventsAscending to populate
ListSessionEventsPageParams.Types with the forwardable event types consumed by
inboundInputsFromPublicSessionEvents, so filtering occurs in the database;
preserve ascending pagination and existing error/termination behavior while
using a larger page size to reduce round trips.
---
Outside diff comments:
In `@tests/sessions_api_test.go`:
- Around line 3841-3878: Update sessionEventQueueEventIDs to left-join
session_events with an explicit e.session_id = s.id binding, while preserving
the tenant and workspace ownership predicates. Make the helper detect a missing
or mismatched event from the nullable left-join result and fail the test instead
of silently omitting the queue row.
---
Nitpick comments:
In `@internal/db/code_sessions.go`:
- Around line 359-372: The queue-delete SQL is duplicated across both call
sites. In internal/db/code_sessions.go lines 359-372, replace the inline
statement in the deletedResult operation with the shared
deleteSessionEventQueueQuery constant; retain internal/db/sessions_sqlx.go lines
127-136 as the sole definition of that predicate.
In `@tests/sessions_api_test.go`:
- Around line 688-1043: Reorder the newly added test functions so failure
scenarios come before success scenarios: place
TestSessionEventQueueDeliveryRollsBackOnInboundFailure,
TestSessionStartupRejectsSecondUserMessage,
TestSessionStartupRejectedBatchHasNoSideEffects, and
TestSessionStartupSerializesConcurrentUserMessages before
TestManagedAgentActivationAtomicallyDeliversSessionEventQueue. Do not change
test implementations or behavior.
🪄 Autofix (Beta)
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: e18f4e05-2362-4958-b95a-b18c0e986c52
📒 Files selected for processing (7)
docs/design/be/session-startup-message-delivery.mdinternal/codesessions/managed_agent_code_session.gointernal/db/code_sessions.gointernal/db/migrations/00038_use_uuid_session_event_queue_tenant_references.sqlinternal/db/session_event_queue.gointernal/db/sessions_sqlx.gotests/sessions_api_test.go
![duckpr[bot]](https://avatars.githubusercontent.com/in/4166808?s=60&v=4){kind=small}
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ 增量复审未发现新问题。
Reviewed changes — 复审了自上次评审（c5a63a4）以来的增量改动，聚焦将激活事务编排从 DB 层上移到 Service 层的重构。
- 上移激活事务编排到 Service 层 — 新增
DB.WithManagedAgentActivationTx/ManagedAgentActivationTx事务句柄，Service.ActivateManagedAgentCodeSessionWithQueue接管跨资源编排（锁 Session → 锁 initializing Code Session → 匹配 queue → 写 inbound → 删 queue → 激活），DB 层只暴露资源级 SQL 方法。符合AGENTS.md中"DB 层只承担数据访问、不编排跨资源业务流程"的边界规则。 - 抽取
appendCodeSessionEventSQLXTx— 将Commit()从事务内 helper 移除，提交责任归还调用方；ManagedAgentActivationTx.AppendCodeSessionInboundEvent复用该 helper，事务边界保持一致。 - 改用 typed error —
SessionArchivedError、SessionStartupMessageConflictError取代 sentinel error，writeSendEventsPersistenceError通过errors.As精确分派 409 / 400。 - 测试同步更新 —
TestManagedAgentActivationAtomicallyDeliversSessionEventQueue、rollback 测试改为通过codeSessionService.ActivateManagedAgentCodeSessionWithQueue调用，验证 Service 层编排路径。
事务语义（加锁顺序、queue 匹配重试、inbound 幂等、queue 行数校验、状态推进）与重构前完全等价；go build ./internal/... 通过。上一轮提出的 environment type 过滤问题（review 4825644921）在本增量中未改动，保持 open。
anthropic/glm-5.2 ｜ 𝕏
…doff Use package sentinel errors for archived and startup-conflict cases, and map Send Events failures with inline errors.Is handlers. Rename activation and queue helpers for clearer handoff (shouldEnqueue, QueueMatches, shouldQueueForStartup), batch queue insert/list with session ownership checks, drop the delete-row-count precondition, and keep the design doc aligned with the code entry points.
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
Caution
新增的 *.gen.go gitignore 规则导致 yourbatis 生成的 mapper 实现无法提交，从干净 clone 执行 go build ./... 会失败。
Reviewed changes — 复审了自上次评审（3fa3cbe）以来的增量改动，聚焦将 Send / 激活路径的 SQL 从手写 sqlx 迁移到 yourbatis 生成的按表 mapper。
- 迁移 Send 与激活 SQL 到 yourbatis mapper — 新增
SessionMapper、SessionThreadMapper、SessionEventMapper、SessionEventQueueMapper、CodeSessionMapper、CodeSessionInboundEventMapper、EnvironmentWorkMapper七个 mapper 接口与对应 XML，通过go generate生成静态 builder 与 scanner。AppendSessionEventsForDelivery和WithManagedAgentActivationTx改用yourbatis.DB.Transaction开启事务。 - 共享同一个 pgxpool —
newDB通过yourbatis.NewDB(database.DB, ...)包装 sqlx 底层*sql.DB，不建立第二个连接池，符合 AGENTS.md 的 sqlx/pgxpool 共享要求。 - 保留 sqlx 用于未迁移事务 — Deployment 创建和 Session 删除仍使用 sqlx 事务链，queue 操作通过
shouldQueueForStartupSQLX/enqueueSessionEventsSQLXTx/deleteSessionEventQueueSQLX保持在同一 sqlx 事务内。 - 重命名事件判定函数 —
forwardPublicEventToWorker→shouldForwardPublicEventToWorker，hiddenWorkerEvent→isHiddenWorkerEvent，publicWorkerOutputEvent→isPublicWorkerOutputEvent，并在shouldForwardPublicEventToWorker中新增user.tool_confirmation类型。 - 新增 mapper builder 单测 —
yourbatis_mappers_test.go覆盖动态 SQL 构建（IN 列表、批量 INSERT、CAST jsonb）、参数绑定顺序和无效 UUID 前置校验。
🚨 yourbatis 生成文件被 gitignore 排除，干净 clone 无法编译
新增的 *.gen.go gitignore 规则会排除 yourbatis 生成的 *.sqlmap.gen.go 文件（如 code_session_mapper.sqlmap.gen.go、session_mapper.sqlmap.gen.go 等），而这些文件包含了代码直接引用的 NewCodeSessionMapper、buildSessionEventMapperListSessionEventsByUUIDs 等符号。从干净 clone 执行 go build ./internal/db/... 报 undefined: NewCodeSessionMapper。
CI 的 lint workflow（.github/workflows/lint.yml）在执行 golangci-lint ./... 前没有 go generate 步骤，因此 CI 会直接失败。设计文档（docs/design/be/session-startup-message-delivery.md:209）明确写着"仓库提交生成文件"，但 .gitignore 与该意图矛盾。
Technical details
# 生成文件被 gitignore 排除导致编译失败
## Affected sites
- `.gitignore:72` — 新增 `*.gen.go` 规则，排除所有 yourbatis 生成的 `*.sqlmap.gen.go`
- `internal/db/managed_agent_activation.go:27` — 引用 `NewCodeSessionMapper`，该函数仅存在于被 gitignore 的 `code_session_mapper.sqlmap.gen.go`
- `internal/db/session_event_queue.go:73` — 同样引用 `NewCodeSessionMapper`、`NewSessionMapper` 等
- `internal/db/yourbatis_mappers_test.go:54` — 测试引用 `buildCodeSessionInboundEventMapperListExistingActivationInboundEvents` 等生成函数
- `.github/workflows/lint.yml:32-34` — CI 直接跑 `golangci-lint ./...`，无 `go generate` 前置步骤
- `docs/design/be/session-startup-message-delivery.md:209` — 明确声明"仓库提交生成文件"
## Required outcome
- 干净 clone（不运行 `go generate`）必须能通过 `go build ./...` 和 CI lint
## Suggested approach
方案 A（与设计文档一致）：删除 `.gitignore` 中的 `*.gen.go`，把生成的 `*.sqlmap.gen.go` 提交到仓库。
方案 B：在 CI workflow 和 pre-commit hook 中添加 `go generate ./internal/db` 前置步骤（但这改变了"仓库提交生成文件"的约定，需要同步更新设计文档）。
## Open questions for the human
- `*.gen.go` 是否有意为其他生成工具添加？如果是，需要用更具体的 pattern（如 `*.sqlmap.gen.go`）避免误排除 yourbatis 文件。Note: 1 inline comment(s) dropped because they did not anchor to lines inside the PR diff:
.gitignore:49(RIGHT) — line 49 (RIGHT) is not inside a diff hunk
anthropic/glm-5.2 ｜ 𝕏
Move inbound/outbound append onto mapperDB.Transaction with table mappers, keeping lock/epoch/idempotency/sequence semantics, rename activation entry to ActivateManagedAgentCodeSession, and update tests and design docs.
CI and local quality gates failed on a clean checkout because *.sqlmap.gen.go is gitignored. Run scripts/generate-go.sh before lint, dead-code, complexity, tests, server restart, and Docker build.
Resolve yourbatis init (#200) conflicts: adopt v0.1.1 with logger-aware db.Open/newDB, drop the accidental local ybatis replace, and keep this branch's session startup error sentinels.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3e211a5c3
ℹ️ 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".
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ 增量复审未发现新问题。上一轮提出的
*.gen.gogitignore 导致干净 clone 编译失败的问题已修复。
Reviewed changes — 复审了自上次评审（c8c0ed0，review 4841932066）以来的增量改动，聚焦 yourbatis 生成文件在干净 checkout 中的可用性修复，以及与 origin/main（yourbatis init #200）的合并冲突解决。
- 在所有 Go 质量门禁前统一生成 yourbatis mapper — 新增
scripts/generate-go.sh（仅go generate ./internal/db），在 CI（lint.yml、dead-code.yml、complexity.yml）、pre-commit（pre-commit-go-lint.sh、go-dead-code.sh、go-complexity.sh）、justfile（test/lint依赖generate-go）、Dockerfile（go build前go generate）和restart-server.sh中统一前置调用。本地验证./scripts/generate-go.sh && go build ./...通过，上一轮undefined: NewCodeSessionMapper已消除。 - pre-commit hook 触发面扩大到生成器输入 —
golangci-lint、go-dead-code、go-complexity三个 local hook 的files正则新增internal/db/*.xml与scripts/generate-go.sh，使 mapper XML 变更能触发重新生成与对应 package 的 lint；pre-commit-go-lint.sh对go.mod/go.sum/scripts/generate-go.sh输入映射到./...全量 lint。 - 合并
origin/main（yourbatis init #200） —f65c999解决冲突：丢弃本地replace ybatis => ...误提交，采用 yourbatisv0.1.1与internal/logging注入的db.Open/newDB，保留本分支的ErrSessionStartupMessageConflict等 session 启动 sentinel error。go build ./...与go vet ./internal/db/... ./internal/codesessions/... ./internal/sessions/...均通过。 - 设计文档同步 —
session-startup-message-delivery.md与development-quality-gates.md更新为"仓库不提交生成文件，scripts/generate-go.sh为统一生成入口"，与本轮选择的方案 B 一致。
｜ View workflow run ｜ Using anthropic/glm-5.2 ｜ 𝕏
anthropic/glm-5.2 ｜ 𝕏
Bring in #202 (stop tracking generated Go mappers). Keep a unified scripts/generate-go.sh entry point (go generate ./internal/db) for just, CI, pre-commit, Docker, and local quality scripts.
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ 增量复审未发现新问题。
Reviewed changes — 复审了自上次评审（f65c999，review 4846232579）以来的增量改动，即 merge origin/main 引入 PR #202 "Stop tracking generated Go mapper sources"。
- 合入
*.gen.go不入库策略 — PR #202 确定了 yourbatis 生成文件不入库的最终方案：.gitignore新增*.gen.go，删除已跟踪的admin_api_keys_mapper.sqlmap.gen.go/console_api_keys_mapper.sqlmap.gen.go。与本分支上一轮选择的方向一致。 - 统一生成入口覆盖到所有门禁 —
scripts/generate-go.sh（go generate ./internal/db）在 CI（lint.yml/dead-code.yml/complexity.yml）、pre-commit（新增go-generatehook + 三个 Go hook 的files正则改用internal/db/*_mapper.xml）、justfile（generaterecipe，test/lint/dead-code/go-complexity/hooks-run均依赖它）、Dockerfile（独立RUN ./scripts/generate-go.sh）和AGENTS.md中统一前置调用。 - 干净 checkout 验证通过 — 本地
./scripts/generate-go.sh && go build ./...成功，上一轮 review 4841932066 报告的undefined: NewCodeSessionMapper编译失败已彻底消除。
｜ View workflow run ｜ Using anthropic/glm-5.2 ｜ 𝕏
anthropic/glm-5.2 ｜ 𝕏
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7c4c908bc
ℹ️ 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".
| existing, err := listExistingActivationInboundEvents( | ||
| ctx, | ||
| tx.codeSessionInboundEventMapper, | ||
| codeSession, | ||
| inputs, |
There was a problem hiding this comment.
When activation replays more than roughly 65,533 distinct forwardable events, listExistingActivationInboundEvents expands every idempotency key into one IN parameter, plus the organization and workspace parameters, exceeding PostgreSQL's 65,535 bind-parameter limit. The later inserts are batched at 500 rows, but this preliminary unbatched lookup fails first, so a large-history session cannot activate and its managed-agent launch is cleaned up; split this lookup into bounded batches as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8819d8624e
ℹ️ 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".
| #{row.EventSubtype}, | ||
| #{row.PayloadUUID}, | ||
| #{row.RequestID}, | ||
| CAST(#{row.Payload} AS jsonb), |
There was a problem hiding this comment.
When YOURBATIS_DEBUG diagnostics are enabled, this binding treats the complete session event payload as an ordinary argument, so user prompts, tool results, or embedded credentials can be rendered into database logs; existing secret-bearing mapper parameters use sensitive=true for this reason. Mark row.Payload sensitive here and in the batch inbound and outbound insert bindings so diagnostics expose only redacted metadata rather than raw request content.
AGENTS.md reference: AGENTS.md:L62-L62
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
ℹ️ 未发现关键问题——仅一处格式 nit 和一个 PR 描述同步建议。
Reviewed changes — 复审了自上次评审（c7c4c90）以来的增量改动。本轮提交是对整个投递机制的架构级重构：彻底移除 session_event_queue 表及其全部 mapper / SQL / helper，改为在 Code Session 激活事务中一次性读取完整 session_events 公开历史并重放入 inbound，通过 Session 行锁在 Send 与 Activate 之间串行化。
- 移除
session_event_queue全部代码与 migration — 删除session_event_queue.go、session_event_queue_mapper.{go,xml}、session_event_queue_sqlx.go以及 migration00047_add_session_event_queue.sql；AppendSessionEventsForDelivery合并回普通AppendSessionEvents，不再区分startup_queued/realtimedelivery。migration00047从未进入main，整文件删除是正确的清理方式，不会产生孤儿表。 - 激活改为完整历史重放 —
ActivateManagedAgentCodeSession锁 Session → 锁initializingCode Session → 读取完整session_events（created_at ASC, id ASC）→ 过滤可转发事件 → 幂等批量写入 inbound → 切active，全部在同一事务。RunnerprepareManagedAgentLaunch不再读取事件快照。 - 行为变化：不再限制单条启动消息 — 旧设计在启动窗口对第二条
user.message返回 409；新设计接受多条启动消息，activation 按历史顺序全部重放。相应测试（TestSessionStartupRejectsSecondUserMessage等）已删除，改为TestManagedAgentActivationReplaysStartupHistory验证两条启动消息的重放顺序。 - 并发正确性验证 — Send 与 Activate 都通过
SELECT ... FOR UPDATE锁同一条 Session 行；cutover 竞态由 inbound 幂等键去重（eventIdempotencyKey基于codeSessionID + direction + payloadUUID，两条路径对同一公开事件产生相同 key）。原 Issue #189 的消息丢失窗口已消除。
ℹ️ PR 描述与当前实现不一致
PR 描述（GitHub PR body）仍然详细描述 queue-based 设计（session_event_queue、409-on-second-message、startup_queued delivery outcome、单条限制），但代码已改为历史重放方案。分支上的设计文档 docs/design/be/session-startup-message-delivery.md 已正确反映新方案，建议同步更新 PR 描述的"方案"、"关键改动"和"行为变化"表格，使其与设计文档一致，避免后续维护者误以为仍有 queue 表和 409 限制。
｜ View workflow run ｜ Using anthropic/glm-5.2 ｜ 𝕏
anthropic/glm-5.2 ｜ 𝕏
| deleted_at | ||
| </sql> | ||
|
|
||
| <select id="ListSessionEventsForActivation" resultType="sessionEventRow"> |
There was a problem hiding this comment.
ℹ️ 这一行的 <select> 标签使用了 Tab 缩进，而文件内其他所有行（以及同级 mapper XML 如 session_mapper.xml）都使用 4 个空格。建议改为 4 空格以保持一致性，避免 pre-commit 的 Prettier / XML 格式化检查报错。
There was a problem hiding this comment.
已在 4d76cdb 中改为 4 空格缩进。
anthropic/glm-5.2 ｜ 𝕏
Rename the locked code-session row variable to avoid confusion with public sessions, document idempotent append branches, and fix mapper XML indentation.
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ 增量复审未发现新问题。激活锁的租户范围加固和分批查重实现正确，编译与测试通过。
Reviewed changes — 复审了自上次评审（f65c999，review 4846232579）以来的增量改动，聚焦激活路径的租户范围加固与分批查重。
- 为激活锁增加
workspace_uuid租户谓词 —LockInitializingCodeSession新增workspaceUUID参数，code_session_mapper.xml的LockInitializingCodeSession查询从WHERE uuid = #{codeSessionUUID}改为WHERE workspace_uuid = #{workspaceUUID} AND uuid = #{codeSessionUUID}。SELECT FOR UPDATE现在在 workspace 范围内锁定，符合 AGENTS.md 中多租户边界显式化的要求。 - 分批查重避免大历史超出参数上限 —
listExistingActivationInboundEvents将 idempotency key 查重改为按managedAgentActivationInboundBatchSize（500）分批执行，避免超长 session 历史触发 PostgreSQL IN 列表参数限制。每批仍保留CodeSessionExternalID交叉校验作为 data integrity guard。 - 移除未使用的
GetLatestCodeSessionStatus— mapper 接口、XML 和生成代码中的GetLatestCodeSessionStatus已全部删除，无残留引用。 - 新增覆盖测试 —
TestTableMappersBuildDynamicQueries/activation_code_session_lock验证锁定 SQL 包含workspace_uuid谓词；TestListExistingActivationInboundEventsBatchesLookup验证 501 个 key 恰好产生 2 次查询调用。
go build ./internal/db/... ./internal/codesessions/... ./internal/sessions/... 通过；TestTableMappersBuildDynamicQueries、TestTableMappersBuildWrites、TestListExistingActivationInboundEventsBatchesLookup 均通过。
｜ View workflow run ｜ Using anthropic/glm-5.2 ｜ 𝕏
anthropic/glm-5.2 ｜ 𝕏
2 participants
Fixes #189
问题
Cloud Session 在 sandbox 启动期间已经可以接收消息，但 Code Session 要到启动链路后半段才创建。
此前 Runner 在
prepare阶段读取一次session_events快照。快照之后、Code Session 创建之前发送的消息虽然已经持久化，API 也返回成功，但它既不在旧快照中，也无法写入尚不存在的code_session_inbound_events，因此 runtime 可能永远收不到。修复方式
session_events继续作为启动输入的唯一事实源，不新增临时 queue、watermark 或公开启动状态。Send Events 和 Code Session activation 使用同一条 Session 行锁串行化：
initializingCode Session，读取完整公开历史，幂等写入 inbound，最后切换为active；Session 对外状态仍为
idle，不新增公开starting状态。启动消息交接流程
flowchart TD Start["Send 与 Activation 竞争同一条 Session 行锁"] Start --> Winner{"谁先获得锁"} Winner -->|Send| SendCommit["提交公开事件到 session_events"] SendCommit --> ActivationReads["Activation 获得锁并读取完整历史"] ActivationReads --> HistoryInbound["幂等写入可转发事件到 inbound"] HistoryInbound --> HistoryActive["Code Session 切换为 active 并提交"] Winner -->|Activation| ActivationCommit["读取历史并写入 inbound"] ActivationCommit --> CodeSessionActive["Code Session 切换为 active 并提交"] CodeSessionActive --> SendAfter["Send 随后获得锁并提交公开事件"] SendAfter --> RealtimeInbound["通过 realtime 写入 inbound"]无论谁先获得锁，成功返回的公开事件最终都会进入该 Code Session 的 inbound：
关键改动
移除 Runner 启动快照
prepareManagedAgentLaunch不再读取或传递InitialEvents；分阶段激活 Code Session
initializing；initializeinbound；created_at ASC, id ASC读取完整session_events；active。保证原子失败语义
限制批量查询和写入
收紧 activation 锁范围
workspace_uuid和 Code Session UUID；统一 Deployment 初始消息
session_events；行为变化
回归测试
TestEnvironmentRunnerDeliversMessageAcceptedBeforeCodeSessionCreation：复现并覆盖 Cloud Session 启动过程中，已成功发送的消息可能到不了 agent #189；TestManagedAgentActivationReplaysStartupHistory：覆盖多条启动消息、顺序和 realtime cutover；TestManagedAgentActivationPreservesLargeHistoryOrder：覆盖超过单批大小的历史顺序；TestManagedAgentActivationRollsBackOnHistoryConversionFailure：覆盖 activation 失败不切 active；TestListExistingActivationInboundEventsBatchesLookup：覆盖幂等键查询分批；success initial user messages replay in order：覆盖 initial events 顺序。本地已验证以上启动期相关测试，以及 lint、dead-code、duplicate-code 和 complexity 门禁。
设计文档