test(store): cover workspace-member paging limit/role/cursor error paths

[KrasimirKralev]

What Problem This Solves

The workspace-member paging helpers in apps/api/internal/store/workspace_member_pages.go (NormalizeWorkspaceMemberPageRequest, DecodeWorkspaceMemberCursor) guard the member-list endpoint against bad input: an out-of-range page limit, an unknown role filter, and a malformed or stale pagination cursor. The existing sqlite/postgres ListWorkspaceMemberPage suites exercise these helpers only on the happy path (valid roles, in-range limits, cursors they round-trip themselves), so the input-validation branches — the ones that decide whether a bad request is rejected or silently mishandled — had no coverage and could regress unnoticed.

Why This Change Was Made

Adds a focused, characterization-only test file (workspace_member_pages_test.go, tests-only, no production change) that pins the error/boundary arms the boundary suites cannot reach: zero-limit default, over-max clamp, negative-limit rejection, invalid role-filter rejection, and cursor rejection for bad base64, non-JSON payloads, a wrong schema version, and a missing tie-break user id. The cases are deliberately the ones a normal paging call never produces, so they complement rather than duplicate the existing integration coverage.

User Impact

No runtime, API, or behavior change. Operators and API clients gain a regression guard: if a future change weakened the limit clamp or the cursor-validation checks, these tests fail instead of the member-list endpoint quietly accepting a malformed cursor or an unbounded page size.

Evidence

Focused run of the new tests on current main, then a mutation control proving each test bites its target branch, on Linux (Go 1.27.1):

$ go test ./apps/api/internal/store/ -run 'WorkspaceMemberPageRequest|DecodeWorkspaceMemberCursor' -count=1 -v
--- PASS: TestNormalizeWorkspaceMemberPageRequest_ZeroLimitDefaults (0.00s)
--- PASS: TestNormalizeWorkspaceMemberPageRequest_OverMaxClamps (0.00s)
--- PASS: TestNormalizeWorkspaceMemberPageRequest_NegativeLimitRejected (0.00s)
--- PASS: TestNormalizeWorkspaceMemberPageRequest_InvalidRoleFilterRejected (0.00s)
--- PASS: TestDecodeWorkspaceMemberCursor_MalformedRejected (0.00s)
--- PASS: TestDecodeWorkspaceMemberCursor_VersionAndUserIDRequired (0.00s)
PASS
ok  	github.com/openclaw/clickclack/apps/api/internal/store	0.004s

$ go test ./apps/api/internal/store/ -count=1   # full package
ok  	github.com/openclaw/clickclack/apps/api/internal/store	0.005s

Mutation controls (each applied to workspace_member_pages.go, test re-run, then reverted): dropping the over-max clamp assignment, disabling the zero-limit default, widening the negative-limit guard, disabling the invalid-role guard, and neutralizing each malformed/stale-cursor return — every mutation turned the corresponding test from PASS to FAIL, and the control was restored byte-identical. gofmt -l and go vet ./apps/api/internal/store/ are clean.

Scope boundary: tests-only (+81/−0, one new file); no production code, dependency, schema, or public-contract change. Allow edits from maintainers is enabled on this branch.

---

This is an AI-assisted contribution (openclaw contribution cron, account @KrasimirKralev).

---

Generated by Claude Code

[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 real behavior proof before merge. Reviewed September 5, 2026, 7:33 AM ET / 11:33 UTC.

ClawSweeper review

What this changes

Adds six Go tests covering workspace-member paging limits, invalid role filters, and malformed or incompatible cursors.

Merge readiness

⛔ Blocked before merge - 2 items remain

Keep open: current main lacks this focused validation coverage, and no actionable patch defect was found. The supplied test evidence does not satisfy the external-contributor real-behavior proof gate.

Priority: P3
Reviewed head: b223af43459f013d67b883dd134a08a506730944

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The tests are focused and source-consistent, but the evidence remains unit-test-only under the required proof standard.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: The patch changes only tests around the shared paging helpers; the captured Go output and mutation claims exercise those helpers in isolation. No real ClickClack member-directory API run demonstrates the covered validation behavior after applying the PR, and test-only changes are not exempt from this external-contributor gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The patch changes only tests around the shared paging helpers; the captured Go output and mutation claims exercise those helpers in isolation. No real ClickClack member-directory API run demonstrates the covered validation behavior after applying the PR, and test-only changes are not exempt from this external-contributor gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	9 items	Verified test-only introduction: The pinned base-to-head diff adds one 81-line test file containing six tests; production code, dependencies, schemas, and workflows are unchanged. The read-only whitespace check passed. Assertions match current behavior: The shared helpers default zero limits, clamp oversized limits, reject negative limits and unknown roles, and reject malformed cursor encoding, JSON, versions, and missing user IDs. Distinct coverage on main: Current-main test searches found no direct tests of either helper. The existing SQLite error assertion covers a cursor/filter mismatch, while inspected HTTP and PostgreSQL tests cover different integration behavior.
Findings	None	None.
Security	None	None.

How this fits together

ClickClack’s member-directory API passes paging inputs through shared validation helpers before querying SQLite or PostgreSQL. These tests protect the helpers that normalize limits and reject invalid filters and cursors.

flowchart TD
  A[Member directory request] --> B[HTTP input parsing]
  B --> C[Shared paging validation]
  T[New regression tests] --> C
  C -->|Invalid input| D[Request error]
  C -->|Valid input| E[Database member query]
  E --> F[Member page and continuation cursor]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The patch changes only tests around the shared paging helpers; the captured Go output and mutation claims exercise those helpers in isolation. No real ClickClack member-directory API run demonstrates the covered validation behavior after applying the PR, and test-only changes are not exempt from this external-contributor gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Complete next step (P2) - Add real API-run evidence for the covered paging validation behavior. Terminal screenshots, copied output, or logs are suitable; redact credentials, private addresses, endpoints, and personal data. Updating the PR body should trigger review automatically; otherwise ask a maintainer to comment @clawsweeper re-review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production versus test delta	Production +0; tests +81; 6 tests	The regression coverage adds no runtime code or upgrade surface.

Technical review

Best possible solution:

Keep validation centralized in the shared store helpers and protect its existing contract with these focused regression tests.

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

Not applicable: this PR characterizes working validation behavior rather than reporting a runtime defect; the asserted outcomes match current source.

Is this the best way to solve the issue?

Yes: direct helper tests are a narrow way to cover validation branches without database setup or production changes, and they complement the existing integration tests.

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

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

Labels

Label changes:

• add P3: This is low-risk regression-test coverage without a reported current runtime failure.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The patch changes only tests around the shared paging helpers; the captured Go output and mutation claims exercise those helpers in isolation. No real ClickClack member-directory API run demonstrates the covered validation behavior after applying the PR, and test-only changes are not exempt from this external-contributor gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

• P3: This is low-risk regression-test coverage without a reported current runtime failure.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The patch changes only tests around the shared paging helpers; the captured Go output and mutation claims exercise those helpers in isolation. No real ClickClack member-directory API run demonstrates the covered validation behavior after applying the PR, and test-only changes are not exempt from this external-contributor gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Verified test-only introduction: The pinned base-to-head diff adds one 81-line test file containing six tests; production code, dependencies, schemas, and workflows are unchanged. The read-only whitespace check passed. (apps/api/internal/store/workspace_member_pages_test.go:15, b223af43459f)
• Assertions match current behavior: The shared helpers default zero limits, clamp oversized limits, reject negative limits and unknown roles, and reject malformed cursor encoding, JSON, versions, and missing user IDs. (apps/api/internal/store/workspace_member_pages.go:76, b223af43459f)
• Distinct coverage on main: Current-main test searches found no direct tests of either helper. The existing SQLite error assertion covers a cursor/filter mismatch, while inspected HTTP and PostgreSQL tests cover different integration behavior. (apps/api/internal/store/sqlite/members_test.go:136, fa52084a04bf)
• Documented contract and API boundary: The directory documents a default limit of 100, maximum 200, supported roles, and opaque cursors. HTTP parsing precedes the store call, and both database implementations use the shared helpers. (docs/features/workspaces.md:49, b223af43459f)
• Latest-release comparison: The supplied v0.4.0 release revision contains the existing paging helper implementation but not the proposed test file; existing runtime behavior is not evidence that this coverage PR is already implemented. (apps/api/internal/store/workspace_member_pages.go:76, e9bfe8ceb8f1)
• Captured contributor evidence: The fully supplied body reports six passing helper tests, a passing package run, mutation controls, formatting, and vet on Linux with Go 1.27.1. It supplies no real member-directory API request/response trace. Captured context sourceRevision: 64403938025581c6694e0887b88026e539312fc5a58c241b21ef165995faf6df. Reviewer execution was not performed. (b223af43459f)

Likely related people:

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

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Add redacted request/response evidence from a real ClickClack member-directory API run on this branch showing paging limits and invalid-input rejection.

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.