Skip to content

feat(runtime-host): bind ScheduledTasks to Connection identity - #4321

Open
testikun wants to merge 1 commit into
apache:mainfrom
testikun:codex/issue-3927-scheduled-task-identity
Open

feat(runtime-host): bind ScheduledTasks to Connection identity#4321
testikun wants to merge 1 commit into
apache:mainfrom
testikun:codex/issue-3927-scheduled-task-identity

Conversation

@testikun

@testikun testikun commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #3927

Scheduled agent_run effects were identified only by slug. 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 + model for 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

  • Core, Storage, Runtime, and Runtime Host builds pass on the repository-supported Node.js version (>=22.19).
  • Core ScheduledTask tests: 733 passed.
  • Storage tests: 1,037 passed, 8 skipped, 0 failed.
  • Runtime Host ScheduledTask production tests: 13 passed.
  • ScheduledTask protocol tests: 5 passed.
  • Coordinator recovery tests: 3 passed on the supported Node.js version.
  • Runtime Host integration coverage verifies that deleting Connection A and recreating its slug as Connection B leaves the legacy task fail-closed without creating a Session or AgentRun.
  • Biome checks and git diff --check pass.
  • The local system Node.js is v20.20.2; tests that require node:sqlite must run with the repository-supported Node.js version and are covered by CI.
  • The full UI build still reports unrelated pre-existing main-branch errors (settledText, autoScroll, trailingAction); this PR adds no new UI type errors.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

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

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Aug 31, 2026

@me2seeks me2seeks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/runtime-host/src/protocol/index.ts Outdated
@testikun
testikun force-pushed the codex/issue-3927-scheduled-task-identity branch from 0d82021 to 19065e0 Compare September 1, 2026 06:14

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@testikun
testikun force-pushed the codex/issue-3927-scheduled-task-identity branch from 91ec3f0 to 439933d Compare September 2, 2026 01:45
@testikun
testikun force-pushed the codex/issue-3927-scheduled-task-identity branch from 439933d to b4ce460 Compare September 2, 2026 01:47
@testikun

testikun commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

@me2seeks @Astro-Han The compatibility-epoch finding is addressed in b4ce4607f: this branch is rebased onto main at 622d6fecd, owns epoch 94, and follows #4308 at epoch 93. The merge conflict is gone and the affected Core/Storage/Runtime Host tests, builds, lint, and format pass locally; the new CI run is in progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(runtime-host): bind ScheduledTasks to immutable Connection identity

3 participants