feat(runtime): proactive-runtime interop bridge to @agent-assistant#96
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces proactive runtime integration into the workforce runtime package by adding a bridge module that maps ChangesProactive runtime bridge integration
Sequence DiagramsequenceDiagram
participant Caller
participant toProactiveSession
participant schedulerBindingFromCtx
participant RuntimeInteropSession
participant ContextSchedulerBinding
participant WorkforceCtx
Caller->>toProactiveSession: WorkforceCtx
toProactiveSession->>RuntimeInteropSession: map workspaceId, agentId, metadata
RuntimeInteropSession-->>Caller: RuntimeInteropSession descriptor
Caller->>schedulerBindingFromCtx: WorkforceCtx
schedulerBindingFromCtx->>ContextSchedulerBinding: wrap adapter for schedule delegation
ContextSchedulerBinding-->>Caller: ContextSchedulerBinding
Note over ContextSchedulerBinding,WorkforceCtx: At runtime
ContextSchedulerBinding->>WorkforceCtx: requestWakeUp(at, payload) → ctx.schedule.at()
ContextSchedulerBinding->>WorkforceCtx: cancelWakeUp(id) → ctx.schedule.cancel()
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+60
to
+62
| * The returned binding stores the supplied adapter; it does not capture | ||
| * `ctx` directly. This keeps the binding usable across event invocations | ||
| * — the handler builds the adapter once and reuses it. |
Contributor
There was a problem hiding this comment.
🟡 Doc claims binding doesn't capture ctx, but adapter closures close over ctx, enabling stale-ctx bugs if reused
The JSDoc at packages/runtime/src/proactive.ts:60-62 states: "The returned binding stores the supplied adapter; it does not capture ctx directly. This keeps the binding usable across event invocations — the handler builds the adapter once and reuses it." However, the adapter's scheduleWakeUp and cancelWakeUp closures at lines 66-78 directly capture ctx via closure (ctx.schedule.at, ctx.agentName, ctx.schedule.cancel). If a caller follows the doc's advice and builds the binding once then reuses it across event invocations (where a fresh ctx is provided each time), the binding will continue routing through the original stale ctx — potentially scheduling against an expired sandbox, wrong workspace, or defunct schedule handle.
Prompt for agents
The JSDoc on schedulerBindingFromCtx (lines 54-63 of packages/runtime/src/proactive.ts) claims the binding does not capture ctx directly and is safe to reuse across event invocations. However, the adapter closures at lines 66-78 close over ctx (ctx.schedule.at, ctx.agentName, ctx.schedule.cancel), so reusing the binding across invocations with different ctx objects would silently use the stale ctx. Either (a) fix the doc to warn callers they must rebuild the binding per invocation, or (b) restructure the adapter so it takes a ctx-getter callback or indirection that allows the ctx to be swapped, making the docs claims true.
Was this helpful? React with 👍 or 👎 to provide feedback.
Adds a thin, opt-in bridge between workforce's WorkforceCtx and the
@agent-assistant/proactive runtime-interop primitives that just shipped
in agent-assistant#91.
What ships
- packages/runtime/src/proactive.ts — two helpers:
- toProactiveSession(ctx, { agentId? }): maps a workforce ctx into the
RuntimeInteropSession descriptor agent-assistant session/memory/
scheduling primitives consume. Defaults agentId to ctx.agentName.
- schedulerBindingFromCtx(ctx): produces a ContextSchedulerBinding
that routes proactive wake-up requests through ctx.schedule.at /
ctx.schedule.cancel. Lets agent-assistant's proactive engine
schedule follow-ups using workforce's own schedule context.
- Re-exports ContextSchedulerBinding, RuntimeInteropSession,
RuntimeScheduleContext for callers building custom adapters.
- packages/runtime/src/proactive.test.ts — 4 tests covering session
shape, agentId override, schedule.at routing, schedule.cancel routing.
- packages/runtime/package.json — adds @agent-assistant/proactive
dependency (^0.4.31) and the ./proactive subpath export.
Why opt-in
- Workforce's runtime doesn't currently use agent-assistant/sessions for
stateful turn tracking — ctx is event-driven and stateless per
invocation. Handlers that compose with agent-assistant tooling import
these helpers; otherwise the runtime stays unchanged. When workforce
adopts session-scoped memory/scheduling, the wiring lifts up into
buildCtx so the bridge becomes implicit.
Notes
- Includes a one-line fix to examples/openclaw-routing.ts pulling
selection.runtime.* → selection.* per #95's PersonaSelection flatten.
Same fix applied by sub-agents on #92 / #94 rebases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
khaliqgant
force-pushed
the
feat/proactive-bridge
branch
from
99bc57b to
8d7ccfc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/runtime/src/proactive.test.ts (1)
97-132: ⚡ Quick winAdd a request→cancel round-trip test for the same binding id.
Right now cancel is validated with a hardcoded id. Using the id returned by
requestWakeUpwould validate the contract end-to-end.Test tweak
test('schedulerBindingFromCtx routes cancelWakeUp through ctx.schedule.cancel', async () => { const cancelled: string[] = []; const ctx = fakeCtx({ schedule: { - async at() { - /* unused here */ + async at() { + /* no-op */ }, async cancel(name) { cancelled.push(name); } } }); const binding = schedulerBindingFromCtx(ctx); - await binding.cancelWakeUp('proactive-reviewer-2026-05-13T09:00:00.000Z'); - assert.deepEqual(cancelled, ['proactive-reviewer-2026-05-13T09:00:00.000Z']); + const id = await binding.requestWakeUp(new Date('2026-05-13T09:00:00Z'), { reason: 'follow-up' } as never); + await binding.cancelWakeUp(id); + assert.deepEqual(cancelled, [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 `@packages/runtime/src/proactive.test.ts` around lines 97 - 132, Update the second test to perform an end-to-end round-trip by calling binding.requestWakeUp to obtain the generated id and then passing that id to binding.cancelWakeUp instead of the hardcoded string: use fakeCtx and schedulerBindingFromCtx to create the binding, call binding.requestWakeUp(at, payload) to capture the returned id, then call binding.cancelWakeUp(id) and assert the cancelled array contains that id; reference the existing helpers schedulerBindingFromCtx, requestWakeUp, cancelWakeUp, and fakeCtx to locate and modify the test.
🤖 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 `@packages/runtime/src/proactive.ts`:
- Around line 66-77: scheduleWakeUp currently returns a synthetic bindingId
generated by bindingIdFor(at, ctx.agentName) but never registers that id with
ctx.schedule.at, so cancelWakeUp calling ctx.schedule.cancel(bindingId) may be a
no-op; update scheduleWakeUp to register the same deterministic id with the
scheduler (for example by passing the bindingId into ctx.schedule.at or calling
a scheduling API that accepts a name) and/or return the actual schedule name
returned by the ScheduleContext so cancelWakeUp can call ctx.schedule.cancel
with the real registered name; adjust cancelWakeUp to use the registered
schedule name (the value returned from ctx.schedule.at or stored mapping)
instead of only the synthetic bindingId, and keep references to scheduleWakeUp,
cancelWakeUp, bindingIdFor, ctx.schedule.at, ctx.schedule.cancel and the
ScheduleContext shape consistent.
---
Nitpick comments:
In `@packages/runtime/src/proactive.test.ts`:
- Around line 97-132: Update the second test to perform an end-to-end round-trip
by calling binding.requestWakeUp to obtain the generated id and then passing
that id to binding.cancelWakeUp instead of the hardcoded string: use fakeCtx and
schedulerBindingFromCtx to create the binding, call binding.requestWakeUp(at,
payload) to capture the returned id, then call binding.cancelWakeUp(id) and
assert the cancelled array contains that id; reference the existing helpers
schedulerBindingFromCtx, requestWakeUp, cancelWakeUp, and fakeCtx to locate and
modify the test.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 40187535-bacb-4fd7-ab4d-6d5f155e5930
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
packages/runtime/package.jsonpackages/runtime/src/proactive.test.tspackages/runtime/src/proactive.tspackages/runtime/src/types.ts
Address PR #96 review feedback: - Doc on schedulerBindingFromCtx incorrectly claimed the binding did not capture ctx and was safe to reuse across event invocations. The adapter closures do close over ctx, so reuse would silently route through stale handles. Doc updated to mark the binding request-scoped. - The synthetic per-timestamp bindingId could not map back to anything registered with ctx.schedule because ScheduleContext.at does not accept a caller-supplied name. Return a stable per-agent slot name (proactive-<agentName>) instead, so a pre-registered persona schedule slot can be cancelled by cancelWakeUp. Cancellation caveat documented. - Tests updated for the new stable slot id. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
12 tasks
# Conflicts: # packages/runtime/src/types.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Spec reference
Source spec: workforce/docs/plans/deploy-v1-schema-cascade-spec.md
Track section: Track E4 — rebase #96 (feat/proactive-bridge)
Final signoff
SIGNOFF_FINAL: COMPLETE Track-E4
Final gate (typecheck + tests)
Self-reflection report
Known gaps after this PR
Co-Authored-By: Ricky deploy-v1 schema cascade noreply@agentworkforce.com