feat: add mock-user-service with 12 user RPC endpoints#175
Merged
Conversation
12 stateless user RPC routes (status/profile/subscription/apps) wired through natsrouter, plus pkg/subject builders, parsers, and wildcards to pin the subject contract for the eventual real implementation. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
Sixth parser for the room-scoped form
chat.user.{account}.request.user.{siteID}.room.{roomID}.{action…},
returning (account, roomID, action, ok). Updates the parser count, tests
section, and removes the related future-work bullet.
https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
Twelve tasks broken into one-commit chunks: three for pkg/subject (builders, parsers, wildcards), six for handler groups, plus main + deploy + client-api docs. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
Twelve concrete subject builders for the user RPC contract — status, profile, subscription, room-scoped subscription, and apps.list. Mirror the existing builder style and live alongside the room/message ones. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
ParseUserSubject for the 8-token area.action form, four narrow wrappers that validate area, and ParseRoomSubject for the room-scoped form whose action tail is joined back into a single string. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
Five area-scoped wildcard builders (status, subscription, profile, room, apps) for services that want one NATS subscription per area instead of one per route. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
Handler struct, request/response types, mock data constants and builders, and the siteID check helper. Handlers themselves are added in follow-up commits. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
status.getByName echoes the requested Name with fixed StatusText / StatusIsShow defaults; status.set is a no-op returning success. Both gate on siteID match. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
profile.getByName echoes the requested Name with fixed DisplayName / Email defaults; gates on siteID match. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
getCurrent, getRooms, getChannels, and getApps each return two mock Subscriptions whose User.Account matches the subject's account param. Filter fields on the request are accepted and ignored. All gate on siteID match. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
subscription.getDM returns a single mock Subscription whose User.Account matches the request's TargetAccount; subscribeApp and unsubscribeApp are no-ops returning Success=true. All gate on siteID match. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
room.subscription.get echoes the roomID from the subject into the returned mock Subscription; apps.list returns two mock apps. Both gate on siteID match. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
main.go parses env config, connects to NATS, builds a Default router with a 5s HandlerTimeout, registers every handler, and shuts down gracefully (router → nc.Drain → tracer). The Register method on Handler subscribes all 12 user RPC subjects in one place. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
Multi-stage Dockerfile, docker-compose entry joining the external chat-local network, and an Azure pipeline adapted from notification-worker (vet → pkg tests → service tests → build → push). https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
Adds section 3.4 covering all 12 mock user-service subjects with their builders, request/response shapes, and the single error case. Flagged dev-only at the top of the section. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
The room-scoped subject is exactly 10 tokens —
chat.user.{account}.request.user.{siteID}.room.{roomID}.{area}.{action} —
not a joined-tail action. ParseRoomSubject now returns
(account, roomID, area, action, ok) and rejects subjects that aren't
exactly 10 tokens. Spec doc updated to match.
https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
Revert the area/action split — callers want a single joined `action`
string (e.g. "subscription.get"). Token count and prefix validation
unchanged: the 10-token room layout
chat.user.{a}.request.user.{s}.room.{roomID}.{area}.{action} is still
the contract, but the parser presents area+action as one field.
https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
action is now just parts[9] (e.g. "get"), not the joined "area.action" tail. Layout validation (10 tokens, room literal at position 6) unchanged. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
📝 WalkthroughWalkthroughAdds a stateless dev-only mock-user-service: subject utilities in ChangesMock User Service
Sequence Diagram(s)sequenceDiagram
participant Client
participant NATS
participant Router
participant Handler
Client->>NATS: request on concrete subject (chat.user.{account}.request.user.{siteID}....)
NATS->>Router: deliver to matching route/pattern
Router->>Handler: invoke handler with parsed account/siteID/action
Handler->>Handler: checkSite(siteID)
alt siteID matches
Handler-->>Router: JSON success reply
else siteID mismatch
Handler-->>Router: typed ErrNotFound("unknown site")
end
Router-->>NATS: send reply
NATS-->>Client: deliver reply envelope (error or success)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
mock-user-service/deploy/docker-compose.yml (1)
1-19: Consider NATS service definition for self-contained local development.This compose file relies on an external
chat-localnetwork where NATS is already running (connecting tonats://nats:4222). While this pattern works for integrated multi-service setups, it means developers cannot rundocker-compose upin isolation for local development without first starting the external NATS infrastructure.For improved local dev experience, consider either:
- Adding a NATS service definition to this compose file with JetStream and HTTP monitoring enabled (as per guidelines:
--jetstream --http_port 8222)- Documenting the prerequisite setup steps (e.g., which compose file must be started first)
- Confirming this matches the project's intended pattern for service-specific compose files
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mock-user-service/deploy/docker-compose.yml` around lines 1 - 19, The compose file for service mock-user-service depends on an external NATS on the chat-local network (see NATS_URL and NATS_CREDS_FILE env vars), which prevents running docker-compose up standalone; add an embedded NATS service to this file (service name e.g. nats) configured with image nats:latest (or project standard), start command flags --jetstream --http_port 8222, expose/map port 4222 and 8222, mount or reference the backend.creds volume used by NATS_CREDS_FILE, and attach it to the chat-local network so mock-user-service can resolve host "nats"; alternatively, if you intentionally rely on external infra, add a short README note documenting the prerequisite compose to start NATS and confirm the intended project pattern.docs/superpowers/specs/2026-05-13-mock-user-service-design.md (3)
61-63: 💤 Low valueAdd language specifier to fenced code block.
Same issue as the previous block - add a language hint for better rendering.
As per coding guidelines, markdownlint-cli2 flagged: "Fenced code blocks should have a language specified (MD040)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-05-13-mock-user-service-design.md` around lines 61 - 63, The fenced code block containing the topic string "chat.user.{account}.request.user.{siteID}.room.{roomID}.subscription.get" is missing a language specifier; update the block by adding a language hint immediately after the opening triple backticks (for example ```text or ```text-plain) so the fenced code follows the MD040 guideline and renders correctly.
181-190: 💤 Low valueAdd language specifier to fenced code block.
Directory tree structure should use
textor similar language hint.As per coding guidelines, markdownlint-cli2 flagged: "Fenced code blocks should have a language specified (MD040)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-05-13-mock-user-service-design.md` around lines 181 - 190, The fenced code block that shows the mock-user-service directory tree (the block starting with "mock-user-service/" and the tree below it) is missing a language specifier; update that triple-backtick fence to include a language hint (e.g., use ```text or ```bash) so the block complies with MD040; no other content changes needed.
55-63: 💤 Low valueAdd language specifier to fenced code block.
Fenced code blocks should include a language specifier for better rendering and tooling support. Since this shows NATS subject patterns (text), consider using
textor leave it as a plain example.As per coding guidelines, markdownlint-cli2 flagged: "Fenced code blocks should have a language specified (MD040)".
📝 Proposed fix
-``` +```text chat.user.{account}.request.user.{siteID}.{area}.{action}</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/superpowers/specs/2026-05-13-mock-user-service-design.mdaround lines
55 - 63, Update the two fenced code blocks containing the NATS subject patterns
so they include a language specifier (e.g., text) after the opening backticks;
specifically modify the blocks that show
"chat.user.{account}.request.user.{siteID}.{area}.{action}" and the room-scoped
example
"chat.user.{account}.request.user.{siteID}.room.{roomID}.subscription.get" to
use ```text at the start of each fenced block to satisfy MD040 and improve
rendering/tooling.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@docs/client-api.md:
- Line 1620: The TOC is missing an entry for the new section "3.4 user-service
(mock)"; add a linked TOC item (matching the header text "3.4 user-service
(mock)") into the existing table of contents block where other section links are
listed so it navigates to the "### 3.4 user-service (mock)" header; ensure the
link text and anchor match the header formatting used elsewhere in the document.- Around line 1627-1628: The mock error example returning {"error":"unknown
site","code":"not_found"} conflicts with the single-field global error-envelope
defined in Section 5; reconcile by updating the global error-envelope to allow
an optional "code" field (e.g., error: string, code?: string) and then update
the mock examples (the reply for mismatched siteID /SITE_ID) and any other
instances (lines showingcode: "not_found") to conform to the new envelope;
alternatively, if you prefer the mock to be the exception, explicitly mark the
mock responses as non-standard in the mock reply examples and keep Section 5
unchanged — pick one approach and apply it consistently across the document.In
@docs/superpowers/plans/2026-05-13-mock-user-service.md:
- Around line 345-354: ParseRoomSubject currently accepts any subject with >=9
tokens and joins the tail into action, but the spec requires exactly 10 tokens;
change the len(parts) check to enforce exactly 10 tokens (i.e., len(parts) ==
- while keeping the rest of the logic (return parts[2] for account, parts[7]
for roomID, and strings.Join(parts[8:], ".") for action) so the function
behavior matches the spec.In
@docs/superpowers/specs/2026-05-13-mock-user-service-design.md:
- Around line 138-144: Clarify the docstring for ParseRoomSubject to
unambiguously state what "action" returns: either the single final token ("get")
or the entire joined tail after roomID ("subscription.get"); update the comment
to reflect the chosen behavior and adjust the example accordingly (e.g., specify
that for subject "chat.user.X.request.user.Y.room.Z.subscription.get" action
returns "get" or "subscription.get"), and ensure the description of return
values (account, roomID, action, ok) uses those exact symbols so readers can
find ParseRoomSubject and its action/roomID returns without ambiguity.In
@mock-user-service/deploy/Dockerfile:
- Line 1: Update the Dockerfile's base image tag: change the FROM instruction
that currently uses "golang:1.25.8-alpine" to the existing tag
"golang:1.25.8-alpine3.23" so the builder stage can pull a valid image; modify
the FROM line in the Dockerfile accordingly.- Around line 13-19: The Dockerfile's final runtime stage runs as root—add a
non-root user and switch to it to satisfy container security (DS-0002): in the
runtime stage that contains ENTRYPOINT ["/mock-user-service"], create or use a
low-privilege user (e.g., add a user/group and chown the binary), ensure
/mock-user-service is owned by that user, and add a USER instruction
before ENTRYPOINT so the process runs unprivileged instead of root.In
@mock-user-service/handler.go:
- Around line 240-254: Replace hardcoded NATS subject strings in
Handler.Register with subject builder functions: implement the following
functions in pkg/subject/subject.go — UserStatusGetByNamePattern(siteID string),
UserStatusSetPattern(siteID string), UserProfileGetByNamePattern(siteID string),
UserSubscriptionGetCurrentPattern(siteID string),
UserSubscriptionGetRoomsPattern(siteID string),
UserSubscriptionGetChannelsPattern(siteID string),
UserSubscriptionGetDMPattern(siteID string),
UserSubscriptionGetAppsPattern(siteID string),
UserSubscriptionSubscribeAppPattern(siteID string),
UserSubscriptionUnsubscribeAppPattern(siteID string),
UserRoomSubscriptionGetPattern(siteID string), UserAppsListPattern(siteID
string) returning the corresponding "chat.user.{account}.request.user.%s...."
format; then update Handler.Register to call natsrouter.Register or
RegisterNoBody with subject.XPattern(h.siteID) (e.g.,
subject.UserStatusGetByNamePattern(h.siteID)) instead of raw string literals for
all routes (use the same pattern for RegisterNoBody entries).In
@pkg/subject/subject_test.go:
- Around line 406-440: Add tests that exercise account token validation: extend
TestParseUserSubject to include cases asserting that builder functions (e.g.
subject.UserStatusGetByName, subject.UserAppsList and any other User* builders
used here) panic when passed account tokens containing '' or '>' and that
subject.ParseUserSubject returns ok=false for subjects whose account token
contains '' or '>'; follow the pattern used in TestParseRoomCreateSubject for
panic/assertion structure and add both negative-builder tests (expect panic) and
negative-parse tests (expect ok==false) for both '*' and '>' tokens.In
@pkg/subject/subject.go:
- Around line 492-512: ParseUserSubject currently returns parts[2] as the
account token without validation; update ParseUserSubject to validate the
extracted account token by calling isValidAccountToken(parts[2]) and return
false (i.e., "", "", "", "", false) if the check fails so only valid account
tokens are propagated; keep all other subject checks intact and return parts[2],
parts[5], parts[6], parts[7], true only when the account token passes
isValidAccountToken.- Around line 546-562: The ParseRoomSubject function returns parts[2] (account)
without validating it; add the same account token validation used in
ParseUserSubject by calling isValidAccountToken(parts[2]) before returning and
if it fails return ok=false. Update the function ParseRoomSubject to check
isValidAccountToken for the account token (parts[2]) and only return the
account, roomID (parts[7]) and action (parts[9]) when that validation passes.- Around line 444-490: All 12 subject builder functions (UserStatusGetByName,
UserStatusSet, UserProfileGetByName, UserSubscriptionGetCurrent,
UserSubscriptionGetRooms, UserSubscriptionGetChannels, UserSubscriptionGetDM,
UserSubscriptionGetApps, UserSubscriptionSubscribeApp,
UserSubscriptionUnsubscribeApp, UserRoomSubscriptionGet, UserAppsList) must
validate the incoming account token using the existing isValidAccountToken
helper before building the subject; add an early guard like: if
!isValidAccountToken(account) { return "" } (or return an explicit error string
per project convention) at the top of each function so wildcard characters (*,) aren’t embedded into NATS subjects. Ensure the check is performed in each
listed function before any fmt.Sprintf is executed.
Nitpick comments:
In@docs/superpowers/specs/2026-05-13-mock-user-service-design.md:
- Around line 61-63: The fenced code block containing the topic string
"chat.user.{account}.request.user.{siteID}.room.{roomID}.subscription.get" is
missing a language specifier; update the block by adding a language hint
immediately after the opening triple backticks (for example ```text orcorrectly. - Around line 181-190: The fenced code block that shows the mock-user-service directory tree (the block starting with "mock-user-service/" and the tree below it) is missing a language specifier; update that triple-backtick fence to include a language hint (e.g., use ```text or ```bash) so the block complies with MD040; no other content changes needed. - Around line 55-63: Update the two fenced code blocks containing the NATS subject patterns so they include a language specifier (e.g., text) after the opening backticks; specifically modify the blocks that show "chat.user.{account}.request.user.{siteID}.{area}.{action}" and the room-scoped example "chat.user.{account}.request.user.{siteID}.room.{roomID}.subscription.get" to use ```text at the start of each fenced block to satisfy MD040 and improve rendering/tooling. In `@mock-user-service/deploy/docker-compose.yml`: - Around line 1-19: The compose file for service mock-user-service depends on an external NATS on the chat-local network (see NATS_URL and NATS_CREDS_FILE env vars), which prevents running docker-compose up standalone; add an embedded NATS service to this file (service name e.g. nats) configured with image nats:latest (or project standard), start command flags --jetstream --http_port 8222, expose/map port 4222 and 8222, mount or reference the backend.creds volume used by NATS_CREDS_FILE, and attach it to the chat-local network so mock-user-service can resolve host "nats"; alternatively, if you intentionally rely on external infra, add a short README note documenting the prerequisite compose to start NATS and confirm the intended project pattern.🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID:
54f275eb-7804-4515-bbcc-c35fc6b3a473📒 Files selected for processing (11)
docs/client-api.mddocs/superpowers/plans/2026-05-13-mock-user-service.mddocs/superpowers/specs/2026-05-13-mock-user-service-design.mdmock-user-service/deploy/Dockerfilemock-user-service/deploy/azure-pipelines.ymlmock-user-service/deploy/docker-compose.ymlmock-user-service/handler.gomock-user-service/handler_test.gomock-user-service/main.gopkg/subject/subject.gopkg/subject/subject_test.go
Comment on lines
+138
to
+144
| // Parses the 10-token room-scoped form | ||
| // chat.user.{account}.request.user.{siteID}.room.{roomID}.{area}.{action} | ||
| // Returns the trailing `{action}` token (e.g. "get" for subscription.get). | ||
| // Returns ok=false if the subject is not exactly 10 tokens or does not | ||
| // start with `chat.user.*.request.user.*.room.*.`. | ||
| func ParseRoomSubject(subj string) (account, roomID, action string, ok bool) | ||
| ``` |
There was a problem hiding this comment.
Clarify ParseRoomSubject action return value.
The comment on line 140 says "Returns the trailing {action} token (e.g. "get" for subscription.get)" but the docstring also describes the action as "the joined tail after roomID". For the route room.{roomID}.subscription.get, should the function return:
- Just
"get"(the final token), or "subscription.get"(all tokens after roomID joined)?
The phrase "e.g. 'get' for subscription.get" suggests the former, but "joined tail" suggests the latter. Recommend clarifying which behavior is intended.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-05-13-mock-user-service-design.md` around lines
138 - 144, Clarify the docstring for ParseRoomSubject to unambiguously state
what "action" returns: either the single final token ("get") or the entire
joined tail after roomID ("subscription.get"); update the comment to reflect the
chosen behavior and adjust the example accordingly (e.g., specify that for
subject "chat.user.X.request.user.Y.room.Z.subscription.get" action returns
"get" or "subscription.get"), and ensure the description of return values
(account, roomID, action, ok) uses those exact symbols so readers can find
ParseRoomSubject and its action/roomID returns without ambiguity.
| @@ -0,0 +1,19 @@ | |||
| FROM golang:1.25.8-alpine AS builder | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify golang:1.25.8-alpine exists in Docker Hub
# Check if the specific Go version exists
docker manifest inspect golang:1.25.8-alpine > /dev/null 2>&1
if [ $? -eq 0 ]; then
echo "✓ golang:1.25.8-alpine exists"
else
echo "✗ golang:1.25.8-alpine not found"
echo "Available 1.25.x versions:"
curl -s 'https://registry.hub.docker.com/v2/repositories/library/golang/tags?page_size=100&name=1.25' | jq -r '.results[].name | select(contains("alpine"))' | head -10
fiRepository: hmchangw/chat
Length of output: 282
Update Dockerfile to use an existing golang:1.25.8-alpine image tag.
The tag golang:1.25.8-alpine does not exist in Docker Hub. Use golang:1.25.8-alpine3.23 instead, which is the available version for Go 1.25.8.
🧰 Tools
🪛 Trivy (0.69.3)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mock-user-service/deploy/Dockerfile` at line 1, Update the Dockerfile's base
image tag: change the FROM instruction that currently uses
"golang:1.25.8-alpine" to the existing tag "golang:1.25.8-alpine3.23" so the
builder stage can pull a valid image; modify the FROM line in the Dockerfile
accordingly.
Addresses PR #175 review: - All 12 user-service builders now panic on account tokens containing NATS wildcards (*, >), matching the existing isValidAccountToken guard used by ParseRoomCreateSubject. - ParseUserSubject and ParseRoomSubject return ok=false on wildcard accounts so malformed inputs don't propagate downstream. - New pattern builders (UserStatusGetByNamePattern, …) emit the natsrouter "{account}" placeholder so callers stop hardcoding subject strings — names and signatures of existing builders/parsers unchanged. - Tests cover all 12 builder panics, both parser rejections, and all 12 pattern outputs. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
Replace the 12 hardcoded subject patterns in Handler.Register with the new subject.User*Pattern(h.siteID) helpers. No behavior change — the patterns expand to the same natsrouter shape, but the contract now lives in pkg/subject. https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
- client-api.md TOC gains the missing 3.4 user-service (mock) link.
- Section 5 error envelope now documents the optional "code" field
emitted by natsrouter-typed errors, reconciling with the mock's
documented {"error","code":"not_found"} on siteID mismatch.
- Plan doc's ParseRoomSubject snippet updated from the obsolete
joined-tail form to the final 10-token strict implementation that
shipped.
https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/subject/subject.go (1)
445-447: ⚡ Quick winHarden account-token validation for
.to preserve subject token boundaries.Line 445 (and the same guard in other builders) blocks
*/>but still allows., which can split the account into extra subject tokens and break parser/pattern matching. If.is not guaranteed impossible at upstream boundaries, include it inisValidAccountToken.Suggested minimal change
func isValidAccountToken(token string) bool { - return token != "" && !strings.ContainsAny(token, "*>") + return token != "" && !strings.ContainsAny(token, "*>.") }Based on learnings: “reject account values containing NATS wildcard characters
*and>… rely on.being absent only if that guarantee is enforced elsewhere; otherwise include.in the guard as well.”Also applies to: 452-454, 459-461, 466-468, 473-475, 480-482, 487-489, 494-496, 501-503, 508-510, 515-517, 522-524
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/subject/subject.go` around lines 445 - 447, The current account-token validation (isValidAccountToken) only rejects NATS wildcards '*' and '>' but still allows '.' which can create extra subject tokens and break matching; update isValidAccountToken to also treat '.' as invalid (i.e., reject any token containing '.', '*', or '>') and ensure all call sites that currently call isValidAccountToken (the various builder guards shown around the panic calls) rely on this updated function rather than duplicating checks so every builder (the guards that panic with "invalid account token: contains NATS wildcard characters") will now also reject tokens containing a dot.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/subject/subject.go`:
- Around line 445-447: The current account-token validation
(isValidAccountToken) only rejects NATS wildcards '*' and '>' but still allows
'.' which can create extra subject tokens and break matching; update
isValidAccountToken to also treat '.' as invalid (i.e., reject any token
containing '.', '*', or '>') and ensure all call sites that currently call
isValidAccountToken (the various builder guards shown around the panic calls)
rely on this updated function rather than duplicating checks so every builder
(the guards that panic with "invalid account token: contains NATS wildcard
characters") will now also reject tokens containing a dot.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f03b817-42af-45bf-a61c-c4129ac9c6d1
📒 Files selected for processing (5)
docs/client-api.mddocs/superpowers/plans/2026-05-13-mock-user-service.mdmock-user-service/handler.gopkg/subject/subject.gopkg/subject/subject_test.go
✅ Files skipped from review due to trivial changes (1)
- docs/superpowers/plans/2026-05-13-mock-user-service.md
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/client-api.md
- pkg/subject/subject_test.go
- mock-user-service/handler.go
mliu33
approved these changes
May 13, 2026
Collaborator
mliu33
left a comment
mliu33
left a comment
There was a problem hiding this comment.
Thanks! Since response format for dm subscription in the room subscription list is different than actual one, you might need to tell chat-frontend to display the DM room name on left sidebar using the actual field returned by real user-service
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
3 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
Implements a stateless development-only service
mock-user-servicethat answers all 12 user RPC subjects with hardcoded responses. Adds corresponding subject builders, parsers, and wildcards topkg/subjectto pin down the contract for future real implementations.Key Changes
pkg/subjectadditions:UserStatusGetByName,UserStatusSet,UserProfileGetByName,UserSubscriptionGetCurrent,UserSubscriptionGetRooms,UserSubscriptionGetChannels,UserSubscriptionGetDM,UserSubscriptionGetApps,UserSubscriptionSubscribeApp,UserSubscriptionUnsubscribeApp,UserRoomSubscriptionGet,UserAppsList)ParseUserSubject,ParseStatusSubject,ParseSubscriptionSubject,ParseProfileSubject,ParseAppsSubject,ParseRoomSubject)UserStatusWildCard,UserSubscriptionWildCard,UserProfileWildCard,UserRoomWildCard,UserAppsWildCard)mock-user-serviceimplementation:main.go: Config parsing (NATS URL, site ID), NATS connection, router wiring, graceful shutdownhandler.go: Handler struct with 12 RPC methods, request/response types, mock data constants and builders, siteID validationhandler_test.go: Table-driven unit tests for all handlers and helpersdeploy/Dockerfile: Multi-stage build (golang:1.25.8-alpine → alpine:3.21)deploy/docker-compose.yml: Service entry on externalchat-localnetworkdeploy/azure-pipelines.yml: Lint/test/build/push pipelinedocs/client-api.md:Implementation Details
pkg/subjectfunctions, never rawfmt.Sprintfin the service.ErrNotFound.Total: 2https://claude.ai/code/session_01PiDXVupNRXSLgbiBz9DsQH
Summary by CodeRabbit
New Features
Documentation
Tests
Chores