Repository navigation
feat: persist confirmed workflow tasks with owner-scoped retries - #2472
Draft
groupthinking wants to merge 30 commits into
Draft
groupthinking wants to merge 30 commits into
groupthinking wants to merge 30 commits into
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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. |
Contributor
✅ E2E Test Results: ALL TESTS PASSED
Test Output |
Contributor
✅ E2E Test Results: ALL TESTS PASSED
Test Output |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical flaws permit cross-origin and fabricated-run task writes, expose credentials over insecure storage connections, and allow unbounded persistent storage growth.
6 open issues
What changed in this PR
Adds durable workflow-task storage to UVAI’s existing reviewed-action API, replacing the in-memory success response. This implements the software slice of #2473; live acceptance and production activation remain outside scope.
Changes:
- Persists validated tasks through Upstash REST with owner/run-scoped IDs, idempotent writes, and required readback.
- Adds authenticated task retrieval and preserves receipts through action-result serialization.
- Extends focused tests and documents verification evidence and remaining acceptance work.
| File | Description |
|---|---|
WORKFLOW_TASK_IMPLEMENTATION.md |
Records implementation details, verification evidence, and acceptance boundaries. |
apps/web/src/lib/workflow-task-store.ts |
Implements session ownership, validation, durable writes, and owner-scoped reads. |
apps/web/src/lib/action-tools.ts |
Replaces in-memory task creation with persistence and failure reporting. |
apps/web/src/lib/action-lifecycle.ts |
Adds structured receipt data to action results. |
apps/web/src/lib/action-agent.ts |
Passes owner context and preserves task receipts. |
apps/web/src/lib/__tests__/workflow-task-store.test.ts |
Tests retries, isolation, identity verification, readback, and storage failures. |
apps/web/src/lib/__tests__/action-tools.test.ts |
Verifies task creation fails without authenticated ownership. |
apps/web/src/app/api/agents/actions/route.ts |
Requires ownership and run IDs for task writes; adds authenticated retrieval. |
apps/web/src/app/api/__tests__/actions-route.test.ts |
Tests authentication, run-ID requirements, and owner-scoped retrieval. |
Review effort: Max
groupthinking
marked this pull request as draft
November 6, 2026 01:04
Apply the six reviewed fixes: request origin protection, signed preview binding, HTTPS/no-redirect storage, per-batch outage fail-fast, explicit owner capacity, and shared KV/Upstash proxy limiter credentials. Local verification: 1631 web tests passed, 54 skipped; typecheck and changed-file ESLint passed. 26 skips are opt-in native Redis cases; this executor blocks Unix sockets, so native Lua concurrency validation remains pending. No live store or production deployment was used. New task IDs fail closed until an operator chooses the capacity limit and initializes verified permanent owner counts under a writer freeze. Existing-ID retries and owner-scoped reads remain supported. Keep draft until native Redis, hosted acceptance and rollout requirements are met.
Contributor
🔍 PR Validation |
Contributor
✅ E2E Test Results: ALL TESTS PASSED
Test Output |
Copilot started work to fix failing Actions workflows on behalf of
groupthinking
November 6, 2026 01:11
Use Undici's public global-dispatcher compatibility adapter for Node 22 native fetch and decode streamed mock request bodies. Restore the prior dispatcher, disable networking, and retain all redirect assertions. Reproduced 7 timeouts on Node 22.23.4 with Undici 8.12.3 before this fix; all 20 transport tests pass after. Negative control enabling redirects fails all 7 redirect cases. Full local suite: 1631 passed, 54 skipped; typecheck and targeted lint pass. Enable the 26 quota integration cases in the existing CI frontend job, which already installs Redis. No service, credential or deployment step is added. Native quota execution remains pending this CI run.
Contributor
✅ E2E Test Results: ALL TESTS PASSED
Test Output |
Carry a cumulative storage deadline across task writes and readback; stop later task writes after uncertain storage results while retaining deterministic retries. Reject mixed KV/Upstash credential families. Add adversarial regressions and update implementation notes. Keep task execution behind signed authenticated review; no production activation.
…on-review-journal.mjs
…y-local-review.mjs
Contributor
✅ E2E Test Results: ALL TESTS PASSED
Test Output |
…her same-site origin can persist attacker-selected plans under the victim’s identity because preview writes lack an origin check. This commit fixes the issue reported at apps/web/src/app/api/agents/actions/route.ts:196-199 ### Trigger and impact A page on another localhost port (or a same-site sibling origin) can send a credentialed `text/plain` POST containing JSON. The route accepts that body, resolves the victim’s ambient cookie identity, and persists the generated plan without checking `Origin`. This allows unsolicited durable journal entries attributed to the victim. The proxy checks authentication and rate limits, but not request origin; the execute branch already explicitly protects against this same-site case. [MDN documents](https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/CORS) that simple `text/plain` POST requests do not require preflight and CORS response restrictions do not prevent their side effects. [SameSite=Lax](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Set-Cookie) still permits cookies on same-site requests. ### Fix and validation The suggested patch applies the existing task-write origin policy before local-journal identity resolution/model work, retaining the cookie-free bearer exception and leaving disabled mode unchanged. A regression using the actual route, bridge and SQLite journal returned 200 before the fix and 403 afterward. All 6 focused Node tests and 26 Python tests pass with the patch. Authentication and provider calls are stubbed in these tests; no browser end-to-end test or full web type-check was run because web dependencies are absent. Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: groupthinking <garveyht@gmail.com>
…xed event history
…te (#2613) Fixes #2469 (integration of the existing review journal; original implementation remains in PR #2470). Integration branch based on PR #2472 (`40dbc6c6`) carrying PR #2470's verified local journal implementation and exact-plan tests. The shared actions route now fails closed in local-review-only mode, refuses execute, persists the exact preview before success, and suppresses executable confirmation tokens. When disabled, PR #2472's signed owner-scoped preview and task persistence behavior remains unchanged. Includes Python journal tests, Node-to-Python SQLite bridge tests, combined-route Node smoke tests, and Vitest route boundary tests. **Draft until post-push CI, security review, and runtime acceptance.** No production deployment, merge, or live execution.
Contributor
✅ E2E Test Results: ALL TESTS PASSED
Test Output |
This branch was successfully deployed
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
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.
Closes #2473 (software implementation only; live acceptance remains unverified).
create_workflow_taskcurrently reports “Created task” without saving anything. This change writes confirmed tasks through the existing Upstash REST configuration, requires session-derived provider/subject identity and a run ID, and verifies readback before reporting fulfillment. Atomic SET NX gives identical retries the same record and ID. Owner-scoped GET readback and task receipt serialization are included; preview remains read-only.Implemented at
b2c935ee0df5af909ba03de009bcac0a99ef6b29against maine93fd0e975cda0f3a59cb7fa812c19c64f6b2a52.Validation: 42 focused tests passed; full web suite 1,530 passed / 28 skipped across 171 files before the final provider/subject identity tightening, then all 42 focused tests passed again. Type-check, changed-file ESLint and diff whitespace checks passed. Tests use an isolated REST fixture: one confirmation plus three concurrent retries produces one record and the same receipt. See WORKFLOW_TASK_IMPLEMENTATION.md for commands, boundaries and remaining acceptance criteria.
This is implementation evidence only. Live Upstash fresh-process readback, verified video source timestamps/hash, authenticated UI acceptance, provider usage/cost and a real end-to-end receipt remain unverified. No merge or production activation is requested. Task writes now require sign-in and a run ID; missing storage fails closed without an in-memory fallback. Existing task records have no TTL. Revert this commit to roll back the code (reverting does not remove stored records).
Related #2470 is a local review-only journal, not this hosted task adapter. Both touch the actions route and need integration review. Hayden authorized this bounded cut by asking to start the Oct 23 status report's recommended next action.