fix: moderators get channel settings that fail with a database error

[sercada]

Moderators were offered channel settings even though channel updates have always required an owner or appropriately scoped bot. Saving returned HTTP 400 with sql: no rows in result set.

Align the settings button with the existing authorization rule and translate denied permission lookups to ErrWorkspaceOwnerRequired in both SQLite and PostgreSQL. Direct moderator/member updates now return HTTP 403. Owner archive/restore and member moderation remain available. Includes regression coverage, regenerated web assets, documentation, and an Unreleased entry thanking @sercada.

Validation:

• Reproduced on current main with TestChannelAdministrationIsOwnerOnlyForPeople and the built server: moderator archive returned 400 with the raw SQL error.
• The regression passes after the fix; PostgreSQL's owner-only regression passes against a local PostgreSQL 18 instance.
• Built-server Chromium tests pass for owner archive/restore and moderator UI/API rejection. Fixtures use disposable synthetic accounts, and fixture commands call the built CLI.
• Full local gate, coverage, and independent P0–P2 review results are recorded in the merge follow-up.

Before, moderator sees channel settings:

After, moderator has no channel-settings button:

Owner still opens channel settings and can archive/restore:

The before capture used the connected Chrome profile; after captures are from the repository's Chromium tests after the Chrome extension disconnected. All data shown is synthetic.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 15, 2026, 1:55 AM ET / 05:55 UTC (Revision 3).

ClawSweeper review

What this changes

The PR hides channel settings from moderators, returns clear permission errors from both database stores, and adds regression coverage, documentation, and refreshed embedded web assets.

Merge readiness

✅ Ready for maintainer review

This remains a useful fix absent from current main and v0.5.0. The supplied proof supports the corrected behavior, and no blocking patch defect was found.

Priority: P2
Reviewed head: c5bed5c78e67cdb8feff1e5691714fe75213687e

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with convincing behavior evidence, complementary regression coverage, and no identified blocking defect.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (live_output): Inspected screenshots demonstrate ChatApp’s corrected settings visibility; the reported built-server runs and previously assessed PATCH/GET transcript support moderator rejection without mutation and successful owner archive/restore.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (live_output): Inspected screenshots demonstrate ChatApp’s corrected settings visibility; the reported built-server runs and previously assessed PATCH/GET transcript support moderator rejection without mutation and successful owner archive/restore.
Evidence reviewed	7 items	Applicable repository policy: Read the complete root AGENTS.md and checked for nested policies; none were found. The patch preserves sqlc queries and generated storedb code, placing error translation in handwritten wrappers. No maintainer-notes directory exists. Current main still contains the mismatch: Main offers channel settings to owners and moderators, while its permission wrapper passes through the database error. The pinned main-to-head diff independently confirms ownership of both repairs. Authorization remains enforced before writes: Both RequireChannelAdmin queries admit owner or bot roles. UpdateChannel checks permission before updating channel rows or inserting events; the HTTP handler separately checks bot scope and workspace. The existing HTTP error mapper translates ErrWorkspaceOwnerRequired to 403. No accepted server principal, schema, or persisted preference changes.
Findings	None	None.
Security	None	None.

How this fits together

ClickClack’s channel settings send updates through its HTTP API to SQLite or PostgreSQL. Server permission checks control whether channel changes and their realtime events are committed.

flowchart TD
  A[Workspace role] --> B[Channel settings visibility]
  B --> C[Channel update request]
  C --> D[API identity and scope checks]
  D --> E[Database permission check]
  E -->|Allowed| F[Save channel and publish event]
  E -->|Denied| G[HTTP 403 response]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Source and test growth	Production +11 net lines; tests +194 net lines	Excluding generated assets and documentation, the small production increase repairs permission presentation and error mapping with focused regression coverage.

Technical review

Best possible solution:

Keep channel controls aligned with existing server permissions and reuse the established permission-error response while preserving owner and scoped-bot access.

Do we have a high-confidence way to reproduce the issue?

Yes: current-main source exposes moderator settings whose permission query rejects their role and whose error becomes HTTP 400. Contributor runtime evidence corroborates this path; this review did not execute it.

Is this the best way to solve the issue?

Yes: the patch repairs the UI mismatch and reuses an existing permission error without changing the server authorization policy or stored data.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 19e4c4e8631e.

Labels

Label justifications:

• P2: Misleading moderator controls and raw database errors are a bounded usability defect; existing server authorization already prevents unauthorized changes.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): Inspected screenshots demonstrate ChatApp’s corrected settings visibility; the reported built-server runs and previously assessed PATCH/GET transcript support moderator rejection without mutation and successful owner archive/restore.
• proof: sufficient: Contributor real behavior proof is sufficient. Inspected screenshots demonstrate ChatApp’s corrected settings visibility; the reported built-server runs and previously assessed PATCH/GET transcript support moderator rejection without mutation and successful owner archive/restore.

Evidence

What I checked:

• Applicable repository policy: Read the complete root AGENTS.md and checked for nested policies; none were found. The patch preserves sqlc queries and generated storedb code, placing error translation in handwritten wrappers. No maintainer-notes directory exists. (AGENTS.md:3, c5bed5c78e67)
• Current main still contains the mismatch: Main offers channel settings to owners and moderators, while its permission wrapper passes through the database error. The pinned main-to-head diff independently confirms ownership of both repairs. (apps/web/src/ChatApp.svelte:260, 19e4c4e8631e)
• Authorization remains enforced before writes: Both RequireChannelAdmin queries admit owner or bot roles. UpdateChannel checks permission before updating channel rows or inserting events; the HTTP handler separately checks bot scope and workspace. The existing HTTP error mapper translates ErrWorkspaceOwnerRequired to 403. No accepted server principal, schema, or persisted preference changes. (apps/api/internal/store/sqlite/sqlc/queries.sql:603, c5bed5c78e67)
• Regression coverage: The introduced tests cover moderator/member HTTP rejection, unchanged channel state, PostgreSQL permission errors, owner archive/restore, role-helper behavior, and browser visibility. The browser fixture uses the real CLI through go run and the configured application server. Tests were inspected, not executed during this read-only review. (tests/e2e/channel-settings.spec.ts:27, c5bed5c78e67)
• Inspected visual and recorded runtime proof: Viewed all three prepared images: the moderator settings gear is present before and absent after, while the owner archive dialog remains available. The captured current body reports successful built-server Chromium and PostgreSQL runs. The completed review at fix: moderators get channel settings that fail with a database error #256 (comment) records an earlier real-server PATCH/GET transcript showing moderator 403 responses, unchanged state, and owner archive/restore; that original transcript is preserved here through the review record rather than independently re-read from the current body. Historical sourceRevision: 83b0d201660680991d5ec28a2eb87c204a4899b6bc678cadb4bb721b84a5cf0d; current supplied sourceRevision: 1900decfbb9053ef2482d26477d2c280435e25649dedf31d707fcd3921c7f089.
• Release and related-work checks: GitHub confirms the current main SHA matches the pinned base and v0.5.0 remains the latest release. Its SQLite permission wrapper still returns the raw query error. The recent PR listing contains no merged replacement for this repair. (apps/api/internal/store/sqlite/sqlite.go, 648202b79b2f)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)

• reviewed 2026-09-13T12:34:39.782Z sha 6dfd883 :: needs real behavior proof before merge. :: none
• reviewed 2026-09-13T13:04:16.267Z sha 6dfd883 :: needs maintainer review before merge. :: none

[steipete]

Merged as 1b5d0a7. Thanks @sercada.

Verified the root cause on main with the new HTTP regression and a built server: moderator channel updates returned 400 with the raw SQL error. The patched HTTP/PostgreSQL regressions pass, and the two built-server Chromium tests prove moderator rejection plus owner archive/restore. The inspected synthetic before/after pictures are in the PR body.

The full local gate completed in stages. pnpm check passed web tests/builds and every Go package except PostgreSQL, which exhausted the package-wide ten-minute budget during fixture commits under local disk/migration-lock contention; no assertion failed. After moving only the disposable PostgreSQL data to tmpfs, go test ./apps/api/internal/store/postgres passed in 37.2s with the normal timeout. pnpm test:fakeco-aws, pnpm test:desktop, pnpm typecheck, pnpm -r typecheck, pnpm lint, and pnpm fmt:check all passed. A duplicate local coverage run was stopped to remove contention; CI's coverage gate passed. No assertions or repository timeout/retry policy were changed.

Independent review was clean through P2. Exact-head CI was green at c5bed5c: Go, TypeScript, Node 24, full Playwright, Docker and all three desktop platforms. No CI rerun was needed.