feat(workflow): journal store + script sandbox#95
Conversation
Create workflow_runs, workflow_events, and workflow_agent_calls with indexes. Effort rename remains v4; journal schema is v5. Co-Authored-By: Claude <noreply@anthropic.com>
Create/claim/cancel/complete runs, monotonic event append+drain, agent call lifecycle, and stale-worker reaping. Wire unit tests. Co-Authored-By: Claude <noreply@anthropic.com>
Parse export const meta (pure literal), compile scripts as vm.Script, and run them with banned Date.now/Math.random/bare new Date plus no process/require/fetch. Rehydrate results into the host realm. Co-Authored-By: Claude <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
| throw new Error("Workflow script did not compile to a function"); | ||
| } | ||
|
|
||
| const result = await withTimeout( |
There was a problem hiding this comment.
[P0] This timeout does not stop synchronous workflow code. The vm.Script timeout only covers creation of the async factory, and this Promise timer cannot fire while factory() is blocking the event loop (for example, while (true) {}). The workflow body needs to run in an externally terminable worker/child process, with a regression test for a synchronous infinite loop.
| consoleProxy: Record<string, (...args: unknown[]) => void>, | ||
| ): Record<string, unknown> { | ||
| return { | ||
| Object, |
There was a problem hiding this comment.
[P0] Injecting host-realm constructors makes this sandbox escapable. I reproduced Object.constructor('return process.version')() successfully recovering the host process. Omitting process/require from globals is therefore insufficient. Please stop exposing host constructors and treat a disposable worker/child process as the actual capability boundary; add constructor-chain escape tests.
| const body = normalized.replace(META_EXPORT, " const meta ="); | ||
|
|
||
| // Reject further imports / exports after transform | ||
| if (/\bimport\s+/.test(body) || /\bexport\s+/.test(body)) { |
There was a problem hiding this comment.
[P1] This scans raw source, so valid strings/comments containing import or export are rejected; agent('Explain export syntax') reproduces it. Please use a parser/tokenizer, or a lexical scan that correctly ignores strings, comments, template literals, and regex literals.
| return { | ||
| events, | ||
| nextSeq, | ||
| terminal: TERMINAL_STATUSES.has(run.status), |
There was a problem hiding this comment.
[P1] terminal can become true while later event pages still exist. A completed run with more events than the page limit makes callers stop after the first page and silently lose the remainder. Please return hasMore/journal-complete separately and only tell a reader it is done when the run is terminal and no events remain after this page.
1 participant
Summary
workflow_runs/workflow_events/workflow_agent_callsWorkflowStorejournal API (create/claim/events/agent calls/reap)vm.ScriptcompileStack
PR2 of 5 · base:
pr/dw-1-types-effortTest plan
tsx src/workflow-store.test.tstsx src/workflow-script.test.tstsx src/workflow-sandbox.test.tsnpm run typecheck