feat(runtime-host): bind ScheduledTasks to Connection identity - #4321
feat(runtime-host): bind ScheduledTasks to Connection identity#4321testikun wants to merge 1 commit into
Conversation
me2seeks
left a comment
There was a problem hiding this comment.
Blocking compatibility issue: this PR declares epoch 84 for the ScheduledTask Connection-identity wire contract, while PR #4308 independently declares the same global epoch 84 for the sandbox-continuation wire contract. RUNTIME_HOST_COMPATIBILITY_EPOCH is a single Host/Client interoperability boundary, not a per-feature version. Both branches are based on the old 9249bf3 base and are currently conflicting with main (which is at epoch 83). Please rebase and either compose both closed-shape changes under one epoch-84 ledger entry if they are intended to ship together, or land one at 84 and bump the other to 85 after the first. The stale 78→79 explanation should be updated as part of the same repair. Until this is resolved, the meaning of epoch 84 depends on merge order and clients cannot be given a deterministic compatibility contract.
0d82021 to
19065e0
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks — reviewed 435d1838 for substance. @me2seeks holds the open block on the epoch, so I've left that to their thread apart from one factual note at the end.
The problem is real and the fix is the right shape. Identifying an agent_run effect by slug alone means a deleted Connection's slug can be re-taken and a stored task then executes under a different account's credentials — a credential-confusion bug, not just a resolution bug. Freezing llmConnectionId + llmConnectionSlug + model, resolving by the immutable ID, and validating the stored slug and model before a Session or AgentRun exists is the correct authority: the ID is the identity and the slug becomes a consistency check rather than a lookup key.
Fail-closed for legacy rows is the right default here. Silently resolving a slug-only task to some Connection is exactly the behaviour being removed, and refusing to execute is recoverable by recreating the task, while running under the wrong credentials is not.
I checked the one thing that looked like it might contradict the description. normalizeExecution rejects a missing llmConnectionId outright, while the type marks it optional for legacy rows, so I expected editing a legacy task's title to fail. It doesn't: normalizeUpdateScheduledTaskInput only normalizes effect when the patch carries it (scheduled-task.ts:215), so non-effect edits never reach that check. The claim in the description holds.
Also confirmed the execution fingerprint covers the full identity rather than the slug, so recovery admission can't re-admit a task under a replacement Connection.
No findings from me on the implementation.
Evidence boundary: I read the effect contract, the normalizer paths, the coordinator's resolution and fingerprint, and the agent_run origin change; I did not run the suites and did not review the 476 lines of new tests in detail.
Factual note, not a verdict — that stays with @me2seeks: the description currently tells three different epoch stories (the English summary says 78 → 79, the Chinese summary says 88 and asks for a specific merge order, the code declares 89), while main is at 87. Worth collapsing to one when you next rebase. On the merge-order point specifically, I'd gently suggest this PR shouldn't need to reference another one at all — taking main + 1 and documenting only its own contract makes it mergeable on its own schedule, whichever lands first.
AI-assisted review: drafted with Maka; I verified the legacy-edit path, the normalizer boundaries, and the fingerprint contents against the branch source myself.
91ec3f0 to
439933d
Compare
Generated-by: Codex
439933d to
b4ce460
Compare
|
@me2seeks @Astro-Han The compatibility-epoch finding is addressed in |
Summary
Fixes #3927
Scheduled
agent_runeffects were identified only byslug. Because slugs can be reused after a Connection is deleted, an existing task could resolve to a different account and execute under the wrong provider credentials.This PR freezes the executable identity to
llmConnectionId + llmConnectionSlug + modelfor new tasks, resolves by the immutable Connection ID, and validates the stored slug and model before creating a Session or AgentRun. Legacy slug-only tasks remain readable and editable for non-effect fields, but fail closed at execution time until explicitly recreated with a Connection identity. Recovery admission also uses a stable execution fingerprint containing the full identity, and the Runtime Host protocol compatibility epoch is set to 94; it follows #4308’s epoch 93 change and must merge after it.The behavior change is intentionally fail-closed: deleting Connection A and creating Connection B with the same slug can no longer cause A's scheduled task to run as B.
中文摘要
修复 ScheduledTask 仅按 slug 识别
agent_run的问题。现在新任务固定保存 Connection ID、slug 和 model,执行时按 ID 解析并校验 slug/model;旧的 slug-only 任务仍可读取和修改非 effect 字段,但执行时会安全失败,不会误用同 slug 的新账号。恢复 admission 也使用包含完整身份的稳定指纹,协议 epoch 在本 PR 中为 94;它专用于 ScheduledTask Connection 身份契约,并应在 #4308 的 epoch 93 sandbox continuation 变更之后合并。Verification
git diff --checkpass.node:sqlitemust run with the repository-supported Node.js version and are covered by CI.settledText,autoScroll,trailingAction); this PR adds no new UI type errors.AI use
Tool(s) and scope: OpenAI Codex designed and implemented the bounded Connection-identity change, added regression coverage, and ran the listed verification. The human contributor remains responsible for review and submission.
Checklist
Does this PR entail a change in behavior?