nigig-org/nimanyatta/KNOWN_ISSUES.md

Known Issues

Tracked from a prior code review; status updated after the Phase 8 hardening pass. Severity and category noted per issue.

Gateway

#	Severity	Category	Issue	Status
1	Critical	Correctness	Double cleanup across three code paths	Fixed — all close paths route through idempotent cleanup_connection
2	Critical	Correctness	send_direct_to after remove_session in logout	Fixed — LoggedOut sent before remove_session
3	Critical	Correctness	Token restore skips session limit enforcement	Verified enforced — session_manager.authenticate evicts prior same-username connections on handshake, in-band login and token-restore paths (in-memory single-connection-per-user policy; the DB-level limit applies to password logins)
4	High	Design	clone_minimal will silently miss new fields	Open (accepted risk — struct is small and central to the hot path)
5	High	Design	Cleanup logic duplicated between task and method	Fixed — DRY'd onto cleanup_connection
6	High	Reliability	Forward task buffer not flushed on shutdown	Open — mitigated by graceful-shutdown sequencing from Phase 7
7	Medium	Design	Stochastic 90% message drop under pressure	Open (backpressure policy decision)
8	Low	Correctness	Rate limiter refill/consume non-atomic	Open
9	Low	Correctness	WS_DUMMY_HASH parameters may not match production	Open
10	Low	Style	as_millis() as u64 silent truncation	Open
11	Low	Style	Captured token_id_for_cleanup used only for logging	Open

Routes / Handlers

#	Severity	Category	Issue	Status
1	Critical	register	Compensating cleanup failure is silent; token generation has no cleanup	Open (mitigated by logging)
2	Critical	sms_verify_otp	TOCTOU on attempt counter despite "FIXED" comment	Fixed — increment_otp_attempts_if_under_limit is now a single atomic conditional UPDATE … WHERE attempts < $max (no read-check-write race). The earlier BEGIN/IF transaction form was a SurrealQL parse error that the fail-closed handler masked as a first-try 429 — found by the Phase 8 route tests, with a regression test (otp_attempts_increment_round_trip)
3	Critical	sms_verify_otp	No session limit for SMS — undocumented and inconsistent	Open (policy decision)
4	High	update_custom_role	Room ownership verified after role priority is used	Open
5	High	Multiple	Redundant DB queries; unscoped fetch used for permission check	Open
6	High	login	SessionLimitExceeded masking leaks timing signal	Open
7	Medium	refresh_token	Malformed User-Agent collapses to "Unknown" fingerprint	Open
8	Medium	set_server_role	Admin bypass undocumented; unnecessary DB call on rejection path	Open
9	Medium	send_message	HTTP handler does unbounded WS fan-out with no error handling	Open
10	Medium	sync	Breaking API change to empty filter semantics without version bump	Documented — empty room_ids filter returns zero rooms; asserted in route tests
11	Low	validate_username	Byte/character count inconsistency (harmless but confusing)	Covered by regression suite (regression_username_byte_vs_char_count)
12	Low	validate_password	Asymmetric byte/char limits undocumented	Covered by regression suite (regression_password_byte_limit_documented)
13	Low	login_types	Hardcoded strings not tied to enum values	Open
14	Low	leave_room	Race condition on owner-member count check	Open
15	—	BroadcastDistributor::clear	Resolved — distributor deleted in Phase 3	—

Scope gaps (Phase-3 chat features from the construction-site scope docs)

Not bugs — functionality from construction-site-app-scope.md / construction-site-app-scope2.md that is not yet implemented and tracked as the post-Phase-8 roadmap:

Feature	Current state
Per-site channel structure (site / management / DM channels)	Rooms are flat, user-created; no site-provisioned channel tree
Pinned document library	PinMessages permission exists; no pin storage or REST/WS handlers
Read receipts on documents	Message receipts via WS MarkRead only; REST read_receipt returns 501 by design
Mentions / message search	mentions field parsed on MessageContent::Text; no delivery/notification or search
Broadcast messages	Not implemented
Moderation (kick/ban enforcement)	KickUsers/BanUsers permission bits + MembershipState::Ban exist; no handlers
Retention policies	Not implemented
Offline queue / inbox	InboxError types exist in nigig-common; server-side offline inbox not wired

WS wire parity: nigig-common (server) vs nimanyatta-protocol (client)

The websocket wire format is now aligned: both sides encode/decode with postcard (to_bytes/from_bytes/decode_all in nigig-common match nimanyatta-protocol; the gateway's fragmentation buffering uses postcard::take_from_bytes so trailing bytes are preserved). AuthMethod (13 variants) is fully aligned in shape and order.

Remaining divergences (pre-existing in the original design — the server's own handlers destructure shapes the client library does not emit, e.g. MessageContent::Text { body, .. } server-side vs Text { text } client-side). Postcard encodes enum variants as positional indices and fields in declaration order, so these gaps change bytes on the wire:

Area	nimanyatta-protocol (client)	nigig-common (server)
ClientToServerMsg	49 variants incl. AddReaction, RemoveReaction, GetPresence, RequestUpload, UploadComplete, GetHistory	31 variants — reactions/uploads/history/presence-query missing; shared subset not in matching order
ServerToClientMsg	81 variants incl. reactions, ReadReceipt, UploadReady/Failed/Progress, History	60 variants — upload/history/reaction families missing
LoginSuccess fields	user_id, access_token, refresh_token, device_id, expires_in	user_id, access_token, device_id, session_id
MessageDeleted	message_id, deleted_at, reason: DeletionReason, deleted_by	message_id, deleted_at_ms, deleted_by (no reason)
MessageContent	Text { text, formatted? }, Media, Reply, Deleted { …, reason }, System, File	Text { body, formatted_body, mentions }, Image/File/Audio/Video (REST JSON contract — pinned by the route tests and load suites)
MessageId	String alias	strong EventId newtype (serialises as the same prefixed string)

Remediation path (roadmap, not a repair): unify both enums on the nimanyatta-protocol shapes (the client library now has byte-pinned characterization tests — treat those as the contract), bump PROTOCOL_VERSION, and add cross-crate wire tests that feed nigig_common::ServerToClientMsg::to_bytes() into nimanyatta_protocol::ServerToClientMsg::from_bytes(). The REST JSON DTOs (SendMessageRequest etc.) should keep their current externally-tagged JSON contract — they are HTTP, not the WS wire.

Notes on earlier "stale suite" breakage

The seven load suites (fast_integration_suite, chat_load_tester_realistic, security_suite, regression_suite, session_limit_test, enhanced_security_tests, enhanced_security) failed to compile for two independent reasons, both fixed in Phase 8:

1. The nigig-common protocol crate they (and the server) depend on had been lost and replaced by mismatched shapes. It was reconstructed as nigig-lite/crates/common from the wire contract in crates/nimanyatta-protocol plus the server's own call sites.
2. Three of the suites are [[bin]] targets whose bodies were #[tokio::test] functions. #[test] items are cfg(test)-gated and are removed from normal (non-test) builds, so those bins had no main and failed with E0601. Phase 8 adds small main entry points and strips the test attributes from the orchestrator functions, so cargo run --bin <suite> --features load works as documented.