Skip to content

feat(world-local): support Hook minimum retention - #2866

Merged
NathanColosimo merged 60 commits into
mainfrom
codex/hook-retention-local
Aug 4, 2026
Merged

feat(world-local): support Hook minimum retention#2866
NathanColosimo merged 60 commits into
mainfrom
codex/hook-retention-local

Conversation

@NathanColosimo

@NathanColosimoNathanColosimo commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • implement experimental_minRetention in the Local World
  • advertise the hookRetention World capability
  • keep terminal Hooks discoverable and their tokens unavailable during retention
  • reject retention beyond 30 days by default with a non-retryable WorkflowWorldError
  • let local testing mirror another World through WORKFLOW_LOCAL_HOOK_RETENTION_LIMIT_DAYS
  • release a Hook and its token immediately when dispose() is called
  • rebuild missing Hook caches from the event log

The core API and event contract are already on main. This PR implements that contract for the Local World; it does not add an external token-release API.

Behavior

experimental_minRetention is measured from Hook creation. It is a minimum:

  • while the run is active, the Hook remains valid even if the deadline has passed
  • after the run becomes terminal, the Hook remains discoverable and its token remains unavailable until the deadline
  • a retained terminal Hook cannot be resumed
  • hook.dispose() removes the Hook and releases its token immediately
  • the Local World accepts up to 30 days by default
  • WORKFLOW_LOCAL_HOOK_RETENTION_LIMIT_DAYS can change that limit so development matches production

Implementation

The absolute tokenRetentionUntil deadline is stored on the hook_created event, the Hook entity, and the existing token claim. The Hook entity is authoritative during terminal cleanup; the event and claim preserve crash recovery without introducing a retained-only data type.

The Local World resolves its retention limit when storage is created. events.create() rejects an oversized hook_created request before acquiring Hook locks or writing an event, Hook, or token claim. The error is a WorkflowWorldError with status 400, so the runtime treats it as deterministic and does not retry it as an infrastructure failure.

Hook claim replacement and release use a bounded per-token filesystem lock because separate Local World processes can share one data directory. The critical section covers only claim recovery and the read-delete-create handoff. A compromised lock fails the current operation instead of throwing from a timer and crashing the process.

When a run completes, fails, or is cancelled, terminal cleanup keeps the Hook and token claim only while retention remains. Hook reads are pure: after the deadline they hide the terminal Hook without deleting storage. The next creator lazily removes the expired claim and claims the token.

The existing per-Hook lifecycle lock still serializes creation, resume, and disposal for one Hook. Independent Hooks in the same run are not serialized.

Postgres explicitly excludes the new Hook field from its schema assertion until its separate retention implementation adds a column and behavior.

Verification

  • dependency build for the Local, Postgres, and Vercel World stack passed
  • @workflow/world-local TypeScript check passed
  • focused Local retention-limit tests passed
  • rejection coverage verifies that no Hook or event state is written
  • coverage also includes completion, failure, cancellation, lost claim persistence, event-log recovery, active runs past the deadline, explicit disposal, compromised locks, pure expiry reads, and concurrent replacement across storage instances

Docs Preview

PagePreview
createHook() minimum retentionv5
World configuration referencev5
Local Worldv5

@vercel

vercelBot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackReadyReadyPreviewAug 3, 2026 11:41pm
example-nextjs-workflow-webpackReadyReadyPreviewAug 3, 2026 11:41pm
example-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workbench-astro-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workbench-express-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workbench-fastify-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workbench-hono-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workbench-nestjs-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workbench-nitro-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workbench-nuxt-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workbench-sveltekit-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workbench-tanstack-start-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workbench-vite-workflowReadyReadyPreviewAug 3, 2026 11:41pm
workflow-docsReadyReadyPreview, v0Aug 3, 2026 11:41pm
workflow-swc-playgroundReadyReadyPreviewAug 3, 2026 11:41pm
workflow-tarballsReadyReadyPreviewAug 3, 2026 11:41pm
workflow-webReadyReadyPreviewAug 3, 2026 11:41pm

@changeset-bot

changeset-botBot commented Jul 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f204f71

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 20 packages
NameType
@workflow/worldMinor
@workflow/world-localMinor
@workflow/web-sharedPatch
@workflow/world-postgresPatch
@workflow/cliPatch
@workflow/corePatch
@workflow/vitestPatch
@workflow/webPatch
@workflow/world-testingPatch
@workflow/world-vercelPatch
workflowPatch
@workflow/buildersPatch
@workflow/nextPatch
@workflow/nitroPatch
@workflow/astroPatch
@workflow/nestPatch
@workflow/rollupPatch
@workflow/sveltekitPatch
@workflow/vitePatch
@workflow/nuxtPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actionsBot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

All tests passed

E2E Test Summary

Summary
PassedFailedSkippedTotal
✅ ▲ Vercel Production145502501705
✅ 💻 Local Development163302271860
✅ 📦 Local Production163302271860
✅ 🐘 Local Postgres162102391860
✅ 🪟 Windows15500155
✅ 📋 Other102402161240
✅ vercel-multi-region270027
Total7548011598707
Details by Category

✅ ▲ Vercel Production

AppPassedFailedSkipped
✅ astro126029
✅ example126029
✅ express126029
✅ fastify126029
✅ hono126029
✅ nextjs-turbopack15104
✅ nextjs-webpack15104
✅ nitro126029
✅ nuxt126029
✅ sveltekit145010
✅ vite126029

✅ 💻 Local Development

AppPassedFailedSkipped
✅ astro-stable129026
✅ express-stable129026
✅ fastify-stable129026
✅ hono-stable129026
✅ nextjs-turbopack-canary136019
✅ nextjs-turbopack-stable15500
✅ nextjs-webpack-canary136019
✅ nextjs-webpack-stable15500
✅ nitro-stable129026
✅ nuxt-stable129026
✅ sveltekit-stable14807
✅ vite-stable129026

✅ 📦 Local Production

AppPassedFailedSkipped
✅ astro-stable129026
✅ express-stable129026
✅ fastify-stable129026
✅ hono-stable129026
✅ nextjs-turbopack-canary136019
✅ nextjs-turbopack-stable15500
✅ nextjs-webpack-canary136019
✅ nextjs-webpack-stable15500
✅ nitro-stable129026
✅ nuxt-stable129026
✅ sveltekit-stable14807
✅ vite-stable129026

✅ 🐘 Local Postgres

AppPassedFailedSkipped
✅ astro-stable128027
✅ express-stable128027
✅ fastify-stable128027
✅ hono-stable128027
✅ nextjs-turbopack-canary135020
✅ nextjs-turbopack-stable15401
✅ nextjs-webpack-canary135020
✅ nextjs-webpack-stable15401
✅ nitro-stable128027
✅ nuxt-stable128027
✅ sveltekit-stable14708
✅ vite-stable128027

✅ 🪟 Windows

AppPassedFailedSkipped
✅ nextjs-turbopack15500

✅ 📋 Other

AppPassedFailedSkipped
✅ e2e-local-dev-nest-stable129026
✅ e2e-local-dev-tanstack-start-129026
✅ e2e-local-postgres-nest-stable128027
✅ e2e-local-postgres-tanstack-start-128027
✅ e2e-local-prod-nest-stable129026
✅ e2e-local-prod-tanstack-start-129026
✅ e2e-vercel-prod-nest126029
✅ e2e-vercel-prod-tanstack-start126029

✅ vercel-multi-region

AppPassedFailedSkipped
✅ nextjs-turbopack2700

📋 View full workflow run

…local
# Conflicts:
#	docs/content/docs/v5/api-reference/workflow/create-hook.mdx
#	docs/content/docs/v5/foundations/idempotency.mdx
#	packages/core/src/workflow/hook.test.ts
#	packages/world-local/src/storage.test.ts
#	packages/world-local/src/storage/events-storage.ts
#	packages/world-local/src/storage/helpers.ts
#	packages/world-vercel/src/events.ts
Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com>
Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com>
@github-actions

github-actionsBot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

📊 Workflow Benchmarks

commit f204f71 · Tue, 04 Aug 2026 00:00:24 GMT · run logs

Backend: vercel · app: nextjs-turbopack

MetricScenarioBest (ms)P75 (ms)P90 (ms)P99 (ms)Samples
TTFSstep1321 (+62%) 🔻1416 🔴 (+28%) 🔻1433 🔴 (+21%) 🔻1448 🔴 (-17%) 💚30
TTFSstream1321 (+389%) 🔻1411 🔴 (+32%) 🔻1441 🔴 (+33%) 🔻1560 🔴 (+38%) 🔻30
TTFShook + stream1364 (+12%)1668 🔴 (+25%) 🔻1692 🔴 (+24%) 🔻1724 🔴 (+20%) 🔻30
STSO1020 steps (inline)177 (+95%) 🔻490 (+214%) 🔻551 (+190%) 🔻759 (+134%) 🔻1016
STSO1020 steps (queue-hop)2310 (-33%) 💚3334 (-3.9%)3334 (-3.9%)3334 (-3.9%)3
WO1020 steps426510 (+185%) 🔻426510 (+185%) 🔻426510 (+185%) 🔻426510 (+185%) 🔻1
SLstream latency105 (+2.9%)173 🔴 (+14%)225 🔴 (+22%) 🔻365 🔴 (+20%) 🔻30
SOstream overhead (text)112 (-23%) 💚208 (-14%)248 (-35%) 💚401 (-8.2%)30
SOstream overhead (structured)116 (±0%)191 (-10%)209 (-11%)346 (+17%) 🔻30
📈 STSO distribution vs main (inline / queue-hop histograms)

1020 steps (inline)

Cumulative STSO time: main 145277ms → this run 416203ms (Δ +270926ms, +186%)

 50-100 ms ┃ main 9 this 0 -9
100-150 ms ┃███████████████████████ main 717 this 0 -717
150-200 ms ┃██████ main 220 this 13 -207
200-250 ms █░┃ main 42 this 91 +49
250-300 ms █░░┃ main 16 this 115 +99
300-350 ms █░░┃ main 7 this 118 +111
350-400 ms █░░┃ main 4 this 133 +129
400-450 ms █░░░┃ main 1 this 161 +160
450-500 ms █░░░┃ main 1 this 161 +160
500-550 ms ░░░┃ main 0 this 118 +118
550-600 ms ░┃ main 0 this 59 +59
600-650 ms ┃ main 1 this 20 +19
650-700 ms ┃ main 0 this 7 +7
700-750 ms ┃ main 0 this 8 +8
750-800 ms ┃ main 0 this 4 +4
800-850 ms ┃ main 0 this 2 +2
850-900 ms ┃ main 0 this 2 +2
900-950 ms ┃ main 0 this 2 +2
950-1000 ms ┃ main 0 this 1 +1
1000-1050 ms ┃ main 0 this 1 +1

1020 steps (queue-hop)

Cumulative STSO time: main 3471ms → this run 8832ms (Δ +5361ms, +154%)

2000-2500 ms ░░░░░░░░░░░┃ main 0 this 1 +1
3000-3500 ms ████████████░░░░░░░░░░░┃ main 1 this 2 +1
📜 Previous results (6)

d3bf2a3

Mon, 03 Aug 2026 21:47:28 GMT · run logs

vercel / nextjs-turbopack

MetricScenarioBest (ms)P75 (ms)P90 (ms)P99 (ms)Samples
TTFSstep237 (-78%) 💚1361 🔴 (+21%) 🔻1417 🔴 (+24%) 🔻1554 🔴 (+26%) 🔻30
TTFSstream241 (-6.2%)1516 🔴 (+37%) 🔻1527 🔴 (+35%) 🔻1574 🔴 (+37%) 🔻30
TTFShook + stream384 (-8.8%)1606 🔴 (+22%) 🔻1703 🔴 (+23%) 🔻1769 🔴 (+17%) 🔻30
STSO1020 steps (inline)178 (+2.3%)499 (-0.8%)559 (+0.9%)769 (+2.8%)1016
STSO1020 steps (queue-hop)1855 (-16%) 💚2653 (-28%) 💚2653 (-28%) 💚2653 (-28%) 💚3
WO1020 steps427379 (-1.1%)427379 (-1.1%)427379 (-1.1%)427379 (-1.1%)1
SLstream latency99 (+6.5%)164 🔴 (+15%) 🔻170 🔴 (+1.2%)202 🔴 (-58%) 💚30
SOstream overhead (text)128 (-5.2%)189 (-22%) 💚250 (-22%) 💚375 (-99%) 💚30
SOstream overhead (structured)126 (-19%) 💚225 (-17%) 💚244 (-21%) 💚1327 🔴 (+68%) 🔻30

6f1e4c2

Mon, 03 Aug 2026 19:16:57 GMT · run logs

vercel / nextjs-turbopack

MetricScenarioBest (ms)P75 (ms)P90 (ms)P99 (ms)Samples
TTFSstep1186 (+351%) 🔻1288 🔴 (+18%) 🔻1303 🔴 (+17%) 🔻1579 🔴 (+19%) 🔻30
TTFSstream1357 (+517%) 🔻1402 🔴 (+37%) 🔻1414 🔴 (+36%) 🔻1440 🔴 (+33%) 🔻30
TTFShook + stream616 (+57%) 🔻1577 🔴 (+26%) 🔻1598 🔴 (+22%) 🔻1712 🔴 (+8.0%)30
STSO1020 steps (inline)162 (-5.3%)461 (-8.0%)513 (-7.1%)720 (-3.0%)1016
STSO1020 steps (queue-hop)2478 (+93%) 🔻3374 (+28%) 🔻3374 (+28%) 🔻3374 (+28%) 🔻3
WO1020 steps390936 (-6.3%)390936 (-6.3%)390936 (-6.3%)390936 (-6.3%)1
SLstream latency84 (-8.7%)137 🔴 (-6.8%)143 🔴 (-17%) 💚296 🔴 (-29%) 💚30
SOstream overhead (text)100 (-15%)141 (-43%) 💚175 (-47%) 💚2357 🔴 (-93%) 💚30
SOstream overhead (structured)103 (-8.8%)138 (-33%) 💚151 (-44%) 💚368 (-8.2%)30

28d27f5

Sun, 02 Aug 2026 01:36:38 GMT · run logs

vercel / nextjs-turbopack

MetricScenarioBest (ms)P75 (ms)P90 (ms)P99 (ms)Samples
TTFSstep249 (-68%) 💚1355 🔴 (+24%) 🔻1378 🔴 (+20%) 🔻1989 🔴 (+31%) 🔻30
TTFSstream252 (+5.9%)1363 🔴 (+26%) 🔻1399 🔴 (+26%) 🔻1445 🔴 (+24%) 🔻30
TTFShook + stream511 (-59%) 💚1609 🔴 (+17%) 🔻1622 🔴 (+13%)1699 🔴 (-7.7%)30
STSO1020 steps (inline)174 (+0.6%)520 (±0%)593 (+1.4%)874 (±0%)1016
STSO1020 steps (queue-hop)1907 (-3.8%)2770 (-16%) 💚2770 (-16%) 💚2770 (-16%) 💚3
WO1020 steps452430 (+1.2%)452430 (+1.2%)452430 (+1.2%)452430 (+1.2%)1
SLstream latency92 (±0%)167 🔴 (-7.2%)187 🔴 (-23%) 💚620 🔴 (+43%) 🔻30
SOstream overhead (text)121 (-11%)190 (-13%)240 (-12%)571 (+52%) 🔻30
SOstream overhead (structured)123 (+4.2%)191 (-20%) 💚247 (-23%) 💚343 (-23%) 💚30

5fad298

Sat, 01 Aug 2026 21:46:55 GMT · run logs

vercel / nextjs-turbopack

MetricScenarioBest (ms)P75 (ms)P90 (ms)P99 (ms)Samples
TTFSstep393 (-50%) 💚1315 🔴 (+20%) 🔻1342 🔴 (+17%) 🔻1366 🔴 (-10%)30
TTFSstream1238 (+420%) 🔻1288 🔴 (+19%) 🔻1294 🔴 (+17%) 🔻1310 🔴 (+12%)30
TTFShook + stream434 (-65%) 💚1556 🔴 (+13%)1582 🔴 (+9.9%)1708 🔴 (-7.2%)30
STSO1020 steps (inline)167 (-3.5%)463 (-11%)521 (-11%)690 (-21%) 💚1016
STSO1020 steps (queue-hop)1524 (-23%) 💚3400 (+2.8%)3400 (+2.8%)3400 (+2.8%)3
WO1020 steps399167 (-11%)399167 (-11%)399167 (-11%)399167 (-11%)1
SLstream latency88 (-4.3%)139 🔴 (-23%) 💚161 🔴 (-33%) 💚428 🔴 (-1.4%)30
SOstream overhead (text)101 (-26%) 💚148 (-32%) 💚168 (-38%) 💚199 (-47%) 💚30
SOstream overhead (structured)104 (-12%)162 (-33%) 💚179 (-44%) 💚722 (+61%) 🔻30

737a7b1

Mon, 27 Jul 2026 22:59:55 GMT · run logs

vercel / nextjs-turbopack

MetricScenarioBest (ms)P75 (ms)P90 (ms)P99 (ms)Samples
TTFSstep1240 (+68%) 🔻1354 🔴 (+20%) 🔻1405 🔴 (+16%) 🔻1751 🔴 (+28%) 🔻30
TTFSstream1267 (+304%) 🔻1344 🔴 (+22%) 🔻1389 🔴 (+21%) 🔻1486 🔴 (+27%) 🔻30
TTFShook + stream1465 (+16%) 🔻1604 🔴 (+14%)1751 🔴 (+20%) 🔻1959 🔴 (+20%) 🔻30
STSO1020 steps (1-20)172 (+1.2%)277 🔴 (-8.3%)434 🔴 (+23%) 🔻690 🔴 (+82%) 🔻19
STSO1020 steps (101-120)184 (-6.6%)278 🔴 (-18%) 💚324 🔴 (-21%) 💚328 🔴 (-34%) 💚19
STSO1020 steps (1001-1020)481 (±0%)557 🔴 (-3.0%)597 🔴 (-7.6%)602 🔴 (-8.4%)19
WO1020 steps396421 (-9.7%)396421 (-9.7%)396421 (-9.7%)396421 (-9.7%)1
SLstream latency104 (+12%)177 🔴 (+5.4%)181 🔴 (-23%) 💚241 🔴 (-4.7%)30
SOstream overhead (text)106 (-19%) 💚174 (-40%) 💚253 (-29%) 💚627 (+49%) 🔻30
SOstream overhead (structured)119 (-12%)172 (-34%) 💚217 (-22%) 💚367 (-11%)30

0057b6c

Mon, 27 Jul 2026 22:09:41 GMT · run logs

vercel / nextjs-turbopack

MetricScenarioBest (ms)P75 (ms)P90 (ms)P99 (ms)Samples
TTFSstep405 (-60%) 💚1424 🔴 (+25%) 🔻1516 🔴 (+28%) 🔻1754 🔴 (+9.1%)30
TTFSstream1267 (+318%) 🔻1403 🔴 (+22%) 🔻1426 🔴 (+19%) 🔻1452 🔴 (+8.3%)30
TTFShook + stream1522 (+237%) 🔻1643 🔴 (+15%)1716 🔴 (+15%) 🔻1920 🔴 (+2.5%)30
STSO1020 steps (1-20)170 (-8.1%)290 🔴 (±0%)394 🔴 (+16%) 🔻759 🔴 (+86%) 🔻19
STSO1020 steps (101-120)214 (+10%)320 🔴 (+10.0%)530 🔴 (-2.9%)550 🔴 (-14%)19
STSO1020 steps (1001-1020)493 (-8.0%)579 🔴 (-14%)606 🔴 (-36%) 💚759 🔴 (-23%) 💚19
WO1020 steps410539 (-7.8%)410539 (-7.8%)410539 (-7.8%)410539 (-7.8%)1
SLstream latency128 (+28%) 🔻165 🔴 (-23%) 💚183 🔴 (-59%) 💚528 🔴 (-6.4%)30
SOstream overhead (text)125 (-2.3%)197 (-18%) 💚212 (-25%) 💚292 (-69%) 💚30
SOstream overhead (structured)131 (+10%)205 (-25%) 💚262 (-11%)436 (+4.1%)30
ℹ️ Metric definitions & methodology

The collapsed STSO distribution section above buckets every step gap of the sequential-steps run (not a sampled window), split by whether the step ending the gap ran inline — in the same warm process as the step before it, so the gap is pure framework overhead — or after a queue-hop — the first step of a fresh process, which pays queue dispatch, client reinit and event-log replay. Bars overlay the two runs: is main, marks where this run lands, bridges the gap when this run has more samples in a bucket.

Best/P75/P90/P99 deltas compare against the most recent benchmark run on main at the time of this run. 🔻 flags a delta worse than +15%, 💚 one better than −15%.

Metrics — TTFS: time to first step body (in-deployment start() → first step body, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) · SO: stream overhead (end-to-end write+consume time beyond the modelled generation window)

Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · stream latency: parallel reader/writer steps on a dedicated stream; SL is the in-deployment write->read propagation (readAt - writtenAt) · stream overhead (text): writer streams 300 variable-length text token deltas paced at 100/s for 3s (a haiku-size LLM's token throughput) while a parallel reader drains the whole stream; SO is the end-to-end write+consume time beyond the 3s generation window (overhead/backpressure) · stream overhead (structured): same workload as stream overhead (text), but each delta is an AI-SDK-style structured object ({ type: 'text-delta', id, text }) instead of a raw string, so the SO gap vs the text scenario is the added serialization cost

🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · SO 250/500/1000

All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor (clientStart) right before start(), so the CI runner’s request and its path through api.vercel.com sit outside every measured window. TTFS = in-deployment start() → first step body (turbo uses the in-process fast path, non-turbo the dispatch path), and includes the VQS dispatch hop plus any /flow cold start. STSO/WO are measured between step bodies on the deployment. SL is measured inside the workflow (parallel reader/writer steps), so it no longer includes the api.vercel.com read path.

Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the /flow invocation for a large fraction of runs, inflating P75+; the Best column shows the fastest (warm-start) sample for comparison.

Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com>
…local
# Conflicts:
#	packages/world-local/src/storage/events-storage.ts

@VaguelySeriousVaguelySerious left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AI review: blocking issues found

Comment threadpackages/world-local/src/storage/helpers.ts Outdated
Comment threadpackages/world-local/src/storage/hooks-storage.ts Outdated
Comment threadpackages/world-local/src/storage/events-storage.ts Outdated
Comment threadpackages/world-local/src/storage/helpers.ts Outdated
Comment threadpackages/world-local/src/fs.ts Outdated
Comment threadpackages/world-local/package.json
Comment threadpackages/core/e2e/e2e.test.ts

@VaguelySeriousVaguelySerious left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AI review: approved.

AI Review: Note

Before merge, both Worlds should enforce a default maximum retention of 30d, overridable by environment variable.

experimental_minRetention is unbounded today. createHook() validates the webhook combination and the World capability, then hands whatever the caller passed to parseDurationToDate with no ceiling, so '10y' or a Date in 2099 is honored verbatim by both backends. Two reasons that matters more here than it looks:

  • Neither World reaps an expired retained Hook on its own. world-postgres never deletes the row unless someone creates a Hook with the same token again; world-local only reaps lazily when a read happens to touch it. An unbounded deadline is therefore effectively a permanent row, and the token stays reserved for as long as it says.
  • The deadline is pinned once at createHook() and re-pinned from the persisted hook_created on replay. That is the right design, but it also means a mistake is durable: there is no later opportunity to correct an over-long value for runs already in flight.

A few things worth settling while wiring the cap:

  • Clamp, don't reject. Silently truncating 90d to 30d is surprising, so the clamp should be visible in a log at minimum.
  • Clamp before the value reaches the event log, so the persisted deadline is the effective one and replay stays deterministic. A consequence to state explicitly: raising or lowering the env var later will not retroactively move deadlines already persisted, which I think is the behavior you want.
  • Use one env var name shared by both Worlds rather than a WORKFLOW_LOCAL_* / WORKFLOW_POSTGRES_* pair, so the two cannot drift, and document it per the repo rule that every user-configurable environment variable is documented.
  • The create-hook docs page should state the default ceiling next to the existing retention section, since "30d" is the example value there and would sit exactly at the limit.

@VaguelySerious

Copy link
Copy Markdown
Member

Use one env var name shared by both Worlds rather than a WORKFLOW_LOCAL_* / WORKFLOW_POSTGRES_* pair, so the two cannot drift, and document it per the repo rule that every user-configurable environment variable is documented.

My agent misunderstood - I think it's fine to have separate variables

@socket-security

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

DiffPackageSupply Chain
Security
VulnerabilityQualityMaintenanceLicense
Addednpm/​@​types/​proper-lockfile@​4.1.41001007980100

View full report

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@NathanColosimo@VaguelySerious