Skip to content

test(web): cover workspace role permission helpers - #149

Draft
KrasimirKralev wants to merge 1 commit into
openclaw:mainfrom
KrasimirKralev:test/web-permissions-role-helpers-coverage
Draft

test(web): cover workspace role permission helpers#149
KrasimirKralev wants to merge 1 commit into
openclaw:mainfrom
KrasimirKralev:test/web-permissions-role-helpers-coverage

Conversation

@KrasimirKralev

Copy link
Copy Markdown
Contributor

Related: N/A (proactive test coverage; no linked issue)

What Problem This Solves

apps/web/src/lib/permissions.ts — the workspace-role UI gating helpers isWorkspaceManager and currentRole — had no colocated test. These helpers decide whether the web client shows manager-only UI and which role applies for the active workspace. A silent regression in them (a role dropped from the manager set, the missing-role fallthrough flipping, or the id/route_id workspace lookup breaking) would mis-gate the client UI with nothing to catch it.

Why This Change Was Made

Adds a node:test suite (permissions.test.ts) alongside the module, matching the existing apps/web/src/lib/*.test.ts convention (node --test, node:assert/strict, type-only imports). It pins the current shipped behavior of both exported helpers across their branches. Non-goal: this is coverage only.

What did NOT change (scope boundary): no change to permissions.ts or any runtime module — the only file added is the test. The tests import the real helpers and assert existing behavior, so they lock it in without altering it.

User Impact

No user-visible change. Developers gain regression protection on the role-gating helpers: the suite fails if the manager role set, the missing-role handling, or the workspace id/route_id lookup regress.

Evidence

Ran the web lib test runner against the new file, then the whole src/lib suite, on fresh main (601224d):

$ node --test apps/web/src/lib/permissions.test.ts
1..5
# tests 5
# pass 5
# fail 0

$ node --test apps/web/src/lib/*.test.ts
# tests 30
# pass 30
# fail 0

Non-vacuous — each assertion pins a real branch. Verified by temporarily mutating permissions.ts one edit at a time (drop moderator from the manager set; invert the !role guard; force MANAGER_ROLES.has()true; drop the route_id arm of the workspace lookup; return a fixed role from currentRole) — every mutation turned at least one test red; source restored byte-identical, suite back to 5/5. oxfmt --check and oxlint both clean on the new file (+51/−0, test-only).

Opened from an org-owned fork via the API; if GitHub's Allow edits from maintainers toggle isn't set on this PR, a maintainer can still push to the branch or supersede-and-land.


Generated by Claude Code

Add node:test coverage for apps/web/src/lib/permissions.ts (isWorkspaceManager,
currentRole), which had no colocated test. Covers manager/non-manager/missing
roles and id/route_id/blank/unmatched workspace lookups.
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Aug 4, 2026
@clawsweeper

clawsweeper Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 4, 2026, 9:22 AM ET / 13:22 UTC.

ClawSweeper review

What this changes

Adds five node:test cases for workspace-role lookup and manager-only UI gating in the web client.

Merge readiness

Ready for maintainer review

This is a correct, focused test-only PR with sufficient terminal proof and no actionable review findings; it is still needed as regression coverage and awaits ordinary maintainer review. Likely related people: Shakker, high confidence.

Priority: P3
Reviewed head: 6339e9fa459b498db1c823705ef6ba8ff04ba4c4

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) A narrow, convention-aligned coverage addition with credible real test-run evidence and no correctness concerns.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR body includes after-fix terminal output showing the new five-test file and the complete web-library suite passing; this test-only change is directly demonstrated by that output.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body includes after-fix terminal output showing the new five-test file and the complete web-library suite passing; this test-only change is directly demonstrated by that output.
Evidence reviewed 5 items Focused clean diff: The PR adds only one 51-line test file; the three-way diff has no whitespace errors.
Current helper contract: The current implementation treats only owner and moderator as managers, returns false for absent roles, and resolves workspaces by either id or route_id; the added assertions cover those branches.
Repository test convention: The web package runs node:test over src/lib/*.test.ts, matching the added colocated test file and imports used by existing library tests.
Findings None None.
Security None None.

How this fits together

The web client receives workspace records and uses role helpers to resolve the active workspace’s role and hide manager-only controls. These UI gates are advisory; the backend remains responsible for authorization.

flowchart LR
A[Workspace API records] --> B[Active workspace identifier]
B --> C[Role lookup]
A --> C
C --> D[Manager role check]
D --> E[Web settings controls]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Test-only scope 1 file added; 51 test lines; 0 production lines changed The patch adds regression coverage without changing runtime authorization or UI behavior.

Technical review

Best possible solution:

Keep the helper contract unchanged and land this colocated suite to preserve its role and route-identifier behavior against future regressions.

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

Not applicable: this is proactive coverage for existing behavior, not a report of a current failure.

Is this the best way to solve the issue?

Yes. A colocated node:test suite follows the existing web-library test runner and exercises the real helpers without adding a competing implementation.

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning high; reviewed against 601224ddee75.

Labels

Label changes:

  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P3: This is a low-risk proactive coverage improvement with no user-visible behavior change.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output showing the new five-test file and the complete web-library suite passing; this test-only change is directly demonstrated by that output.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output showing the new five-test file and the complete web-library suite passing; this test-only change is directly demonstrated by that output.

Evidence

What I checked:

Likely related people:

  • Shakker: Introduced the workspace permission helpers in the workspace-settings feature and owns the current blamed implementation through the embedded-web refresh. (role: introduced behavior and recent area contributor; confidence: high; commits: 26792729cb59, a571a1de695f; files: apps/web/src/lib/permissions.ts, apps/web/src/routes/app/[workspaceID]/(settings)/settings/+layout.svelte)

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-08-04T08:32:31.229Z sha 6339e9f :: needs maintainer review before merge. :: none
  • reviewed 2026-08-04T12:01:25.373Z sha 6339e9f :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant