Wire workspace admin controls#54
Conversation
There was a problem hiding this comment.
Pull request overview
This PR wires full workspace admin controls end-to-end: backend store + HTTP API for updating workspace profile, transferring ownership, and deleting a workspace; plus web UI updates to manage and render workspace icons. It also updates the TypeScript SDK build to be Windows-friendly.
Changes:
- Add workspace admin API endpoints (PATCH workspace, transfer ownership, delete) backed by new store interface methods and permission checks.
- Persist and surface
workspace.icon_urlacross DB schemas/migrations, store adapters, API responses, and web UI icon rendering. - Replace POSIX-only
mkdir/cpin the TS SDK build script with a Node-based copy script.
Reviewed changes
Copilot reviewed 27 out of 73 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/sdk-ts/scripts/copy-generated-types.mjs | Node script to copy generated OpenAPI types (Windows-compatible). |
| packages/sdk-ts/package.json | SDK build now invokes Node copy script instead of POSIX shell commands. |
| apps/web/src/styles/sidebar.css | Styles for workspace header icon and guild rail image sizing/cropping. |
| apps/web/src/styles/settings.css | Styles for workspace settings controls, status/errors, selects, and avatar image fit. |
| apps/web/src/routes/app/[workspaceID]/settings/overview/+page.svelte | Workspace settings overview now supports editing name/slug, uploading icon, transferring ownership, and deleting workspace. |
| apps/web/src/lib/types.ts | Adds Workspace.icon_url. |
| apps/web/src/components/navigation/Sidebar.svelte | Sidebar header can render workspace icon. |
| apps/web/src/components/navigation/GuildRail.svelte | Guild rail renders workspace icon images when present. |
| apps/web/src/ChatApp.svelte | Passes selected workspace icon URL into Sidebar. |
| apps/api/internal/webassets/dist/index.html | Rebuilt embedded webassets HTML entrypoint. |
| apps/api/internal/webassets/dist/200.html | Rebuilt embedded webassets fallback HTML. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/9.CDu5alQM.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/9.BDSCeL1M.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/8.DsFZpWZS.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/8.BHcl3rtv.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/7.9an27izZ.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/6.CagJg1eD.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/5.zX4VRxlb.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/5.O5p3Pyje.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/4.D73AL4lC.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/4._3UejRZu.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/3.F-rapJUn.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/3.BOBxDKYZ.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/2.Bkcfcy6u.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/10.D5B2o53Y.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/10.BPYd-zrK.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/1.DanbpO9Q.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/1.CHMiefCn.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/0.DiWWjBeQ.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/nodes/0.BriggHg7.js | Rebuilt embedded webassets bundle output. |
| apps/api/internal/webassets/dist/_app/immutable/entry/start.Dyv7sipE.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/entry/start.2f8svVwH.js | Rebuilt embedded webassets start entry. |
| apps/api/internal/webassets/dist/_app/immutable/entry/app.RH0QXgiE.js | Rebuilt embedded webassets app entry. |
| apps/api/internal/webassets/dist/_app/immutable/entry/app.DAXQSUER.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/chunks/YqczlggC.js | Rebuilt embedded webassets chunk output. |
| apps/api/internal/webassets/dist/_app/immutable/chunks/ngo_HMNL.js | Rebuilt embedded webassets chunk output. |
| apps/api/internal/webassets/dist/_app/immutable/chunks/DXYf4c9c.js | Rebuilt embedded webassets chunk output. |
| apps/api/internal/webassets/dist/_app/immutable/chunks/CXzWvqht.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/chunks/C3nQAG0S.js | Rebuilt embedded webassets chunk output. |
| apps/api/internal/webassets/dist/_app/immutable/chunks/BMwWeE0w.js | Rebuilt embedded webassets chunk output. |
| apps/api/internal/webassets/dist/_app/immutable/chunks/BiKJNDVR.js | Rebuilt embedded webassets chunk output. |
| apps/api/internal/webassets/dist/_app/immutable/chunks/BhIAEoqt.js | Rebuilt embedded webassets chunk output. |
| apps/api/internal/webassets/dist/_app/immutable/chunks/BeJijayk.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/_app/immutable/chunks/B-UA2BLZ.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/webassets/dist/app/immutable/chunks/aWd3JpF.js | Removed/rotated hashed bundle output from rebuild. |
| apps/api/internal/store/types.go | Adds ErrWorkspaceOwnerRequired, Workspace.IconURL, and new store interface methods for workspace admin mutations. |
| apps/api/internal/store/sqlite/storedb/queries.sql.go | Updates generated sqlite storedb rows to include icon_url in workspace queries. |
| apps/api/internal/store/sqlite/storedb/models.go | Adds icon_url field to sqlite storedb Workspace model. |
| apps/api/internal/store/sqlite/sqlite.go | Adds owner permission helper and workspace settings normalization/slug conflict mapping. |
| apps/api/internal/store/sqlite/sqlc/schema.sql | Adds workspaces.icon_url column to sqlite schema. |
| apps/api/internal/store/sqlite/sqlc/queries.sql | Extends workspace queries + adds new workspace mutation/permission queries (sqlc source). |
| apps/api/internal/store/sqlite/sqlc_adapters.go | Maps icon_url from sqlc rows into store Workspace. |
| apps/api/internal/store/sqlite/routes.go | Includes icon_url in resolved route target workspace payload. |
| apps/api/internal/store/sqlite/mutations.go | Implements UpdateWorkspace / TransferOwnership / DeleteWorkspace in sqlite store. |
| apps/api/internal/store/sqlite/migrations/0021_workspace_icon_url.sql | SQLite migration to add icon_url. |
| apps/api/internal/store/sqlite/members.go | Workspace-by-slug query now includes icon_url. |
| apps/api/internal/store/postgres/storedb/queries.sql.go | Updates generated postgres storedb rows to include icon_url in workspace queries. |
| apps/api/internal/store/postgres/storedb/models.go | Adds icon_url field to postgres storedb Workspace model. |
| apps/api/internal/store/postgres/sqlc/schema.sql | Adds workspaces.icon_url column to postgres schema. |
| apps/api/internal/store/postgres/sqlc/queries.sql | Extends workspace queries + adds new workspace mutation/permission queries (sqlc source). |
| apps/api/internal/store/postgres/sqlc_adapters.go | Maps icon_url from sqlc rows into store Workspace. |
| apps/api/internal/store/postgres/routes.go | Includes icon_url in resolved route target workspace payload. |
| apps/api/internal/store/postgres/postgres.go | Adds owner permission helper and workspace settings normalization/slug conflict mapping. |
| apps/api/internal/store/postgres/mutations.go | Implements UpdateWorkspace / TransferOwnership / DeleteWorkspace in postgres store. |
| apps/api/internal/store/postgres/migrations/0014_workspace_icon_url.sql | Postgres migration to add icon_url. |
| apps/api/internal/store/postgres/members.go | Workspace-by-slug query now includes icon_url. |
| apps/api/internal/httpapi/server.go | Registers new workspace admin routes and handlers; maps owner-required errors to 403. |
Files not reviewed (4)
- apps/api/internal/store/postgres/storedb/models.go: Generated file
- apps/api/internal/store/postgres/storedb/queries.sql.go: Generated file
- apps/api/internal/store/sqlite/storedb/models.go: Generated file
- apps/api/internal/store/sqlite/storedb/queries.sql.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 65a76f89e7
ℹ️ 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".
|
Codex review: needs real behavior proof before merge. Reviewed July 12, 2026, 8:11 AM ET / 12:11 UTC. Summary Reproducibility: not applicable. to the feature request itself. The earlier pagination, authorization, cleanup, and concurrency defects have concrete source paths and focused regression coverage on the exact head; the current hosted failure reproduces only an unchanged notification test outside this feature. Review metrics: 4 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Keep the current atomic authorization and durable-cleanup design, re-run or classify the notification test failure, and attach redacted exact-head browser plus terminal/database evidence for existing-profile migration, ownership transfer, permanent deletion, cleanup failure persistence, restart, and recovery. Do we have a high-confidence way to reproduce the issue? Not applicable to the feature request itself. The earlier pagination, authorization, cleanup, and concurrency defects have concrete source paths and focused regression coverage on the exact head; the current hosted failure reproduces only an unchanged notification test outside this feature. Is this the best way to solve the issue? Yes for the implementation direction: typed SQL mutations, explicit owner/manager boundaries, serialized Postgres writes, and a durable cleanup queue are the narrowest coherent design. Merge readiness still requires better inspectable lifecycle proof and a clean or explained browser run. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against eb6e05d0dd73. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (11 earlier review cycles; latest 8 shown)
|
Solvely-Colin
force-pushed
the
s/workspace-admin-controls
branch
from
65a76f8 to
970edae
Compare
clawsweeper
Bot
added
proof: 📸 screenshot
Solvely-Colin
force-pushed
the
s/workspace-admin-controls
branch
from
970edae to
2fd181b
Compare
|
Resolved the review blockers in 2fd181b:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
Solvely-Colin
force-pushed
the
s/workspace-admin-controls
branch
from
2fd181b to
ad0467f
Compare
clawsweeper
Bot
removed
rating: 🦐 gold shrimp
Solvely-Colin
force-pushed
the
s/workspace-admin-controls
branch
from
ad0467f to
72db3ae
Compare
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
rating: 🐚 platinum hermit
Solvely-Colin
force-pushed
the
s/workspace-admin-controls
branch
from
f2e3152 to
842a100
Compare
clawsweeper
Bot
added
proof: 📸 screenshot
steipete
force-pushed
the
s/workspace-admin-controls
branch
from
842a100 to
9d859b3
Compare
|
Exact-head landing proof for
Hosted checks are now running against this exact head. @clawsweeper re-review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d859b332e
ℹ️ 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".
| IconURL: body.IconURL, | ||
| }) | ||
| if err == nil && event.ID != "" { | ||
| s.publishEvent(r.Context(), event) |
There was a problem hiding this comment.
When one member changes the workspace name or icon, this publishes workspace.updated, but the web realtime handler in ChatApp.svelte only reloads for channel and member-moderation events and never handles this event type. Other connected members therefore keep seeing stale GuildRail/Sidebar names and icons until a full reload, so the new admin controls do not propagate to live clients; please reload workspace data on workspace.updated.
Useful? React with 👍 / 👎.
| if !strings.HasPrefix(contentType, "image/") { | ||
| return errors.New("workspace icon must be an image") | ||
| } |
There was a problem hiding this comment.
When an API or SDK client sets an upload with a type like image/svg+xml or image/heic as the icon, this validation accepts it, but getUpload later runs the stored type through safeUploadContentType, which only preserves PNG/JPEG/GIF/WebP image MIME types and otherwise serves application/octet-stream. That lets a workspace save an icon URL that the sidebar/rail <img> can render as broken; validate against the same renderable image allowlist here and in the SQLite mirror, or serve the accepted types as images.
Useful? React with 👍 / 👎.
|
Hosted notification-race stabilization is now on exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 020b7f84e1
ℹ️ 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".
| if targetRole == store.WorkspaceRoleBot || targetRole == store.WorkspaceRoleGuest { | ||
| return store.Workspace{}, store.Event{}, errors.New("new owner must be a human member or moderator") | ||
| } |
There was a problem hiding this comment.
When the selected target is currently blocked or timed out, this validation still allows the transfer because it only checks the role; the moderation row remains in place after the target is promoted. The new owner is then immediately rejected by owner/manager flows that call requireNoModerationBlockTx (for example workspace updates and member moderation), while the UI can still offer such members because the transfer list is built from the regular member page without moderation state. Please reject or clear active moderation for the target before promoting them; the SQLite implementation mirrors this same check.
Useful? React with 👍 / 👎.
|
Merged by rebase at
No release, deployment, version bump, tag, registry publication, or unrelated dependency update was performed. |
|
Post-merge verification on synchronized |
Summary
Exact candidate
020b7f84e133e4f1c567c8ce7c66c3826cebc072eb6e05d0dd73872a3ba1d3822c1a7ad1d6a8fb4aProof
CLICKCLACK_POSTGRES_TEST_DSN=<disposable-postgres-17> pnpm check— pass, including SQLite and real Postgres migrations and lifecycle tests.pnpm coverage— pass, 85.8% total Go statement coverage.pnpm test:e2e— pass, 61 Playwright tests.go run github.com/goreleaser/goreleaser/v2@v2.16.0 check— pass.pnpm audit --prod— zero vulnerabilities.govulncheck ./...— no vulnerabilities found.User-visible behavior
Screenshot