upstream merge 2026-05-08 PR 7: host-service v2 canonical workspace.create + attachment store (PR1+PR2)#462
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (125)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b10ec5c03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…(PR1) (superset-sh#3893) * docs(plans): add v2 workspace create canonical refactor specs Two design docs guiding a clean reimplementation of the canonical workspace.create() flow: - 20260425-canonical-workspace-create-flow.md: umbrella design covering the unified host-service create API, host-scoped attachments, pane store registry, prompt boundary, and PR sequencing (PRs 1-7). - 20260425-host-agent-configs-pr1.md: PR 1 spec for the host-runtime agent config model (host_agent_configs table + settings router). Sourced from the prior v2-workspace-create-canonical branch so the reimplementation can land PR-by-PR per the plan. * docs(plans): switch host agent configs to argv-array launch spec Replace the single `launchCommand` string with a structured argv-array shape (`command` + `args[]` + `promptArgs[]` + `env`) matching VS Code ITerminalProfile / Tabby Shell / WezTerm SpawnCommand / Zellij panes. Empty launches drop `promptArgs` automatically, so codex/opencode/copilot no longer carry their prompt-mode flags into no-prompt sessions. Storing argv directly avoids shell-quoting bugs and makes prompt injection a list push instead of string concatenation. Adds first-class `env` overlay and the per-preset breakdown for the 8 in-scope agents. Implementation in PR1 will be redone against this shape in a separate branch; this branch is now docs-only. * docs(plans): address review on prompt contract + PR1 scope canonical-workspace-create-flow.md: - Make `prompt` required on agent launches in both code blocks (was optional in the API shape but later text said agent launches require one and promptless = raw terminal). Reconciled to the stricter rule and added a Notes line pointing readers to the Agent Configs section. 20260425-host-agent-configs-pr1.md: - Tighten the Summary to make explicit that this PR ships only the storage + tRPC router + V2 settings page. V2 modal and launch dispatch remain on legacy desktop presets and move to host configs in PR 5. * wip: canonical workspace create flow + optimistic attachment uploads Consolidate in-flight create lifecycle into one WorkspaceCreatesManager + useWorkspaceCreates hook (submit/retry/dismiss). Navigate immediately on submit; route renders creating/error/notfound states from the in-flight store. Sidebar reads the same store and supports dismiss on hover for failed entries. Optimistic attachment uploads via a module-scoped Zustand store keyed (fileId, hostUrl). Files upload to whichever host was active when added; switching hosts hides their pills without re-uploading and keeps cached attachment ids for return visits. Submit awaits in-flight uploads and joins file metadata for error messaging. Submit-time gating moved into handleSubmit so all paths (button, Enter, Cmd+Enter) respect preconditions. Plain Enter inserts a newline; submit is button or Cmd+Enter only. Server-side: workspaces.create detects an existing standard-path worktree and adopts it instead of failing on git worktree add — folds the picker's adopt path into the same submit. Layout for v2-workspace falls through to the page when no real row exists so the in-flight UI can render. Other cleanup: drop legacy in-flight-creates store + reconciler, the v2 useCreateWorkspace wrapper, useAdoptWorktree; collapse picker checkout+adopt callbacks; UploadingAttachmentPill replaces the library pill with subtle status overlays; PR link command keeps the Show closed checkbox (pagination concern); useShallow on the draft selector to fix an infinite-loop bug. * fix: drop simple-git --quiet on rev-parse and ship membership claim - refExists/localBranchExists/remoteExists: simple-git's `raw` resolves successfully with empty stdout when --quiet is on, so every "new branch" was being treated as already existing. Drop the flag and validate a 40+ hex sha was actually printed. - auth: include the user's full membership list in OAuth access-token claims so cross-org JWT checks pass downstream. - org-resource-access: split the not-found / wrong-org error paths so org-mismatch reports the actual ids in the diagnostic. - pull-requests: skip refresh for workspaces whose worktree was deleted on disk; simple-git would otherwise throw a confusing directory-does-not-exist error. - git/utils: stop logging on missing remote/origin — both null cases are expected and callers handle them. * chore(cli): override env vars from workspace .env in dev script Without -o, dotenv-cli leaves any pre-existing process.env vars alone, so a shell-level `SUPERSET_HOME_DIR=~/.superset` (set globally for the production CLI) wins over the workspace's `.env` value and dev CLI ops land in the prod data dir instead of `superset-dev-data/`. * feat(cli): add agents run command + variadic flag support - cli-framework: extend `.variadic()` to string flags so repeated `--flag value` invocations accumulate into an array. Drop the auto-`isRequired: true` from `.variadic()` — both existing positional callsites already chain `.required()` explicitly, no behavior change. - cli: new `superset agents run --workspace <id> --agent <preset|id> --prompt <text> [--attachment-id <uuid>]...` for spawning an agent inside an existing workspace. Wraps the host service `agents.run` mutation; resolves the workspace's host via the cloud lookup. * refactor: server-generate workspace name + per-side AI rename gate The renderer no longer generates a friendly-random fallback. Both `name` and `branch` are now optional on the create input — the server picks a friendly random for whichever side is missing, and the AI rename only replaces the side(s) the user didn't supply. - `workspaces.create` schema: drop `autogenerateName`, make `name` optional, allow both `branch` and `pr` to be absent (refine relaxed to "not both set"). Server generates+dedupes a friendly branch name when `branch` is undefined; falls back to PR title or branch for workspace name when `name` is undefined. - `applyAiWorkspaceRename`: take `renameTitle` / `renameBranch` flags; only apply the side the caller asked for. Both `true` from the manual `aiRename` mutation; computed per-create-input on the branch path; skipped entirely on the PR path. - Renderer: drop `friendlyFallback` from the draft store + context + `PromptGroup` placeholder; `resolveNames` returns `string | null`; `useSubmitWorkspace` sends only what the user typed (no synthetic name). In-flight states show "Creating workspace" with no subtitle when the name is absent. - plans: add the v2-workspaces-create-test-plan covering this PR's divergences from v1. * docs(plans): add comprehensive input × state matrix for workspaces.create Enumerates every input combination (PR vs branch mode, typed vs absent name/branch, taskIds, explicit id) crossed with the relevant pre-existing on-disk / DB state (idempotency, adoption, tag, remote-only ref, base-branch existence). Plus the failure/rollback paths and the post-refactor AI-rename behavior table. Drives the smoke-test plan and surfaces the regressions vs v1. * refactor(host-service): inline AI naming in workspaces.create Move AI naming (workspace title + branch) inline so it lands in the same v2Workspace.create round-trip rather than firing a post-create rename + Electric resync. Also overlap host.ensure with the git work. Restructure: - `host.ensure` kicks off at the top, awaited inside `registerCloudAndLocal` — keeps the cloud round-trip off the critical path. - AI naming kicks off in parallel when the user supplied a prompt but left at least one of (name, branch) blank. - Auto-gen branch path: `Promise.all([aiPromise, resolveNewBranchStartPoint, listBranchNames])` — branch name = AI suggestion (deduped), falling back to a friendly random when the LLM is absent or fails. No generated-then-renamed branch and no `git branch -m`. - Typed branch path: `Promise.all([planBranchSource, aiPromise])`. Existing-branch detection still uses planBranchSource; AI title rename can race with that lookup. - PR path skips AI naming entirely (PR title + derived branch are meaningful). Bench (presentations repo, p50, 5 runs each): typed branch, no prompt: 2400ms (no LLM) typed branch, with prompt: 2044ms (AI title only) auto-gen branch, with prompt: 1858ms (AI both) Inline-LLM is essentially free — the LLM (~700ms) overlaps with the ~900ms `git fetch`/`resolveStartPoint` step. v2_workspaces gets the right name from the start; no friendly-name flash. * fix(host-service): wire PR-checkout recovery into workspaces.create The recovery utility (`recoverPrCheckoutAfterGhFailure`) shipped on main via superset-sh#3725, but was wired into the old `workspaceCreation.checkout` procedure that this PR deleted. Without recovery, `workspaces.create` fails for any merged PR whose head branch has been deleted from the remote — a common case post-merge. Plug the same recovery flow into the new PR-create catch block: - fetch prMetadata.headRefOid via `gh pr view` - on `gh pr checkout` failure, try the synthetic-pr-ref fallback or FETCH_HEAD recovery (utility decides based on the error) - rollback + throw only when recovery declined; otherwise continue past the catch with the worktree on FETCH_HEAD * fix(host-service): adopt PR-pre-existing local branch when OID matches When the local repo already has the PR's head branch (typical after a prior `gh pr checkout` outside Superset), compare its OID to `prMetadata.headRefOid`: - match → adopt the existing branch into a new worktree (no `gh pr checkout`, no `--force`, no data-loss risk) - diverge → CONFLICT with a specific message naming both OIDs Removes the unconditional CONFLICT for stale-but-clean PR branches — the dominant case for engineers who review teammates' PRs locally. Diverged-branch case still blocks (user has commits the PR doesn't). * feat(db): regenerate workspace_tasks migration as 0044 after merge Main landed 0043_submitted_prompts so our original 0043_add_workspace_tasks collided. Regenerated via `drizzle-kit generate` so it slots in cleanly on top of main's 0043. * chore: drop pr-test.ts dev bench script * chore: drop unused _navigate in DashboardSidebarWorkspaceItem * feat: stitch linked-context bodies into agent prompt + workspace task M:1 - Client-side prompt builder fetches PR/GitHub-issue/internal-task bodies optimistically on link-time and stitches them into the agent prompt at submit. Bodies are cached in a module-scoped Zustand store keyed by source:id so close-and-reopen of the modal reuses fetched content. The agent now spawns whenever ANY context (typed prompt, linked PR, linked issue, linked task, attachments) is present, and the host-side schema is relaxed to allow attachment-only launches. - Split the GH content fetches off the workspace-creation router into pullRequests.getContent and a new issues.getContent (issuesRouter) so the names reflect what they are (general PR/issue body fetches) rather than what flow first needed them. - Replace the workspace_tasks join table with a nullable task_id FK column on v2_workspaces. The product is one-task-per-workspace; M:N was over-engineered. Collapses linkTask + unlinkTask into one setTask({ workspaceId, taskId | null }) mutation. Updates the renderer, host service, mcp-v2, sdk, and trpc routers accordingly. Migration 0044 regenerated as add_task_id_to_v2_workspaces. - Drop completed plan / spec docs that were carried on this branch. --------- Co-authored-by: Satya Patel <satyapatel111@gmail.com>
* feat(host-service): host attachment store (v2 PR 2) PR 2 of the canonical workspace.create() refactor — see plans/20260425-canonical-workspace-create-flow.md. Adds host-scoped attachment storage so the renderer can upload files once (keyed only by an opaque attachmentId) and reference them in later agent launches without re-uploading or shuttling bytes through workspace creation. Backend - New `attachments.upload` and `attachments.delete` tRPC procedures. - Storage is per-org under `<HOST_MANIFEST_DIR>/attachments/<id>/`, matching where `host.db` lives. This gives clean GC when an org is removed and one isolation boundary across the host service. - Each attachment is stored as `<id>.<ext>` plus a `metadata.json` sidecar (id, mediaType, originalFilename, sizeBytes, createdAt). - Type validation uses the `mime-types` lib to map MIME → extension — any MIME the lib recognizes is accepted (no curated allowlist). - Hard-coded 25 MB per-file cap. - `attachmentId` is a UUID; `delete` enforces UUID input shape, which also blocks path-traversal via the id field. - `delete` is intentionally idempotent (silent success on missing) so cleanup callers don't need try/catch. - Renderer never sees the on-disk path; `attachmentId` is the only stable identifier that crosses the wire. Tests (9 tests against a temp dir as HOST_MANIFEST_DIR): - upload writes bytes + metadata to disk - correct extension chosen per MIME (txt, pdf, jpg, json) - unrecognized MIME rejected - empty payload rejected - oversized payload rejected - unique id per upload - delete removes the directory - delete is idempotent for unknown id - non-UUID id rejected (path traversal guard) Out of scope (deferred to later PRs): - Renderer state slice for tracking uploaded ids (PR 2 follow-up; not needed until the new workspace modal in PR 5). - Direct/streaming upload endpoint — base64-over-tRPC is the v1 transport per the plan. - Resolving attachmentId → host-readable path inside `workspace.create()` prompt assembly. That lives in PR 4. Adds dep: `mime-types@3.x` + `@types/mime-types`. * docs(plans): add PR 2 plan for host attachment store Captures the scope, public API, on-disk layout, validation, and explicit out-of-scope list for this PR. Notes why storage is per-org under HOST_MANIFEST_DIR and why there's no hand-curated MIME allowlist. Renderer state slice is deferred to PR 5 (the modal that consumes it). * fix(host-service): address review tier 1 on attachments attachments.ts: - Reject oversized payloads BEFORE Buffer.from allocates the decoded buffer. Adds estimateDecodedBase64Bytes(); a 1GB base64 string no longer spikes ~750MB of host memory before being rejected. - Drop the dead try/catch around Buffer.from. Node/Bun's base64 decode silently skips invalid characters and never throws — the catch was misleading dead code. Decoded-empty bytes are still caught by the bytes.length === 0 branch. - Stop wrapping `bytes` in `new Uint8Array(...)` before writeAttachment. Buffer already extends Uint8Array; the wrap was a redundant copy. storage.ts: - Treat blank/whitespace-only HOST_MANIFEST_DIR as unset. The previous `?.trim() ?? fallback` preserved empty strings, which would have resolved to `attachments/` (relative to CWD). Tests: - Split the conflated empty-payload test into three distinct cases: single-byte accept, schema-layer reject of empty input string, and the previously-uncovered decoded-empty branch (input `"="` passes zod min(1) but Buffer.from returns 0 bytes). - New oversized test exercises the pre-decode estimator instead of allocating a real 25MB buffer in test memory. - New regression tests for blank and whitespace-only HOST_MANIFEST_DIR fallback to ~/.superset/host/standalone/. Plan doc: noted per-org storage quota as a v1 follow-up (matches the review feedback we're explicitly deferring). * test(host-service): assert stored attachment bytes match upload Read the persisted file back and compare to the decoded upload bytes so a truncated or corrupted write would fail the happy-path test.
MocA-Love
force-pushed
the
upstream/batch-2026-05-pr7-host-service-v2-pr1
branch
from
d43f423 to
40a4ad5
Compare
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
MocA-Love
pushed a commit
that referenced
this pull request
May 8, 2026
Recorded as integrated via -s ours after batch PRs #455-#464. Taken via individual PRs: - PR 1 (#455): v2 polish 前半 safe set (9 commits) - PR 2 (#456): v2/host-service polish 中盤 (12 commits) - PR 3 (#457): sidebar polish + jwt API (5 commits) - PR 4 (#458): host-service tRPC retry/cache/timeout (3 commits) - PR 5 (#459): v2 diff pane / file pane polish (2 commits) - PR 7 (#462): host-service v2 canonical workspace.create + attachment store (PR1 superset-sh#3893 + PR2 superset-sh#3916) - PR 11 (#463): agents API + onboarding (7 commits + 1 cleanup) - PR 12 (#464): v1→v2 import flow rewrite (11 commits + 2 follow-ups) - PR 13 (#460): host-service shell env probe + typo (2 commits) - PR 16 (#461): marketplace 19 themes (1 commit) Skipped / deferred (recorded as integrated for behind=0): - PR 6: CLI v1 launch (superset-sh#3898 + 30+ CLI/SDK followups) — defer to dedicated migration - PR 9: v2 PR3 (superset-sh#3940) + revert (superset-sh#4017) — net-zero pair - PR 10: pty-daemon (superset-sh#3896, superset-sh#3971, superset-sh#4054) — fork keeps its terminal-host - PR 14: Slack MCP-v2 (superset-sh#4197, superset-sh#4208) — depends on mcp-v2/sdk divergence - PR 15: onboarding remaining (superset-sh#4115, superset-sh#4125, superset-sh#4214, superset-sh#4213, superset-sh#4222, superset-sh#4225) — depends on fork's deleted setup pages Behind: 0 after this merge.
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
upstream 同期バッチ第 7 弾。host-service v2 canonical workspace.create + host attachment store (2 commits combined: PR1 superset-sh#3893 + PR2 superset-sh#3916)。
進捗: PR 1-5 + PR 13 + PR 16 マージ済み (34 / 223)、本 PR で +2 commits。
取り込み内容
PR1 と PR2 を combined にした理由: PR1 が renderer 側で
client.attachments.upload.mutate(...)を参照する useUploadAttachments を追加するが、対応する router (attachments) は PR2 で追加される。typecheck がスタンドアロンで通らないため一括取り込みとした。Fork 側のコンフリクト解決（主要点）
packages/trpc/src/router/v2-workspace/v2-workspace.ts: fork の組織/header context を維持packages/host-service/src/trpc/router/router.ts: fork の独自 routers を保持しつつ upstream のattachments,workspace-creation(refactored),workspace-cleanup等を統合apps/desktop/src/renderer/.../PromptGroup.tsx+ 周辺 hooks: fork のuseSubmitWorkspace,DashboardNewWorkspaceDraftContext, custom render path を維持しつつ upstream の host_agent_configs / agentLaunchKind / 新 prop shape に追従useBranchPickerController.ts/useBranchContext.ts/CompareBaseBranchPicker.tsx: fork のデフォルトBranchFilter = "branch"を upstream の enum ("all" | "worktree") に合わせて"all"に変更biome.jsonc: upstream のpackages/sdk/src/client.ts(15+ noExplicitAny / noNonNullAssertion warning) を fork の strict lint から ignore（upstream-managed コードのため fork では緩める方針）Fork 固有機能ヘルスチェック
githubExtendedprocedure 健在除外した commits（次の PR で）
9e03b4917 #3914host agent configs (v2 PR 1, argv-array shape) — 後続として別 PR でa07e446be #4140gh CLI first-class — workspace-creation 系の依存が複雑、PR 8 後にTest plan
bun install --force成功Note
/Volumes/sub/superset-dmg/upstream-merge-2026-05/に PR7 ラベルで保存予定