Alarm pipeline wiring + full code-review backlog resolution #119
Assignees
dohertj2 (dohertj2)
Clear assignees
dohertj2 (dohertj2)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: dohertj2/mxaccessgw#119
Reference in New Issue
Block a user
Delete Branch "docs/alarm-client-wm-app-finding"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
This branch carries two bodies of work.
1. Alarm pipeline (
docs/alarm-client-wm-app-finding)commands,
WorkerAlarmRpcDispatcher, session auto-subscribe, ack-by-name(
Provider!Group.Tag), and end-to-end live smoke verification.2. Full code-review resolution
A tracked, per-module code review under
code-reviews/, then resolution ofall 128 findings across 11 modules.
findings.md+ generatedREADME.mdindex,regen-readme.pytooling, and
REVIEW-PROCESS.md.every module.
(Critical); worker poll-timer STA safety, immediate first heartbeat, and
dropped-reply diagnostics (High); orphan-worker termination, API-key scope
validation, Galaxy-refresh resilience (Medium); plus Low-severity
performance, documentation, and convention fixes across the gateway,
worker, contracts, and all five language clients.
GalaxyHierarchyRefreshServiceTeststiming race, astale
WorkerProjectReferenceTestsassertion, and regenerated theout-of-sync Java client protobuf code.
Verification
MxGateway.slnbuilds clean — 0 warnings (TreatWarningsAsErrorson).MxGateway.Tests408/408 ·MxGateway.Worker.Testsx86231/231 ·
MxGateway.Client(.NET) 73 ·Client.Python68 · Go/Java/Rusttoolchains clean.
🤖 Generated with Claude Code
Gap 1 — WorkerPipeSession now passes `eq => new AlarmCommandHandler(eq)` as the alarmCommandHandlerFactory in all three places it constructs MxAccessStaSession (two convenience constructors and InitializeMxAccessAsync). Previously the parameterless MxAccessStaSession() set the factory to null, so every SubscribeAlarms / AcknowledgeAlarm / QueryActiveAlarms command returned "alarm consumer not configured" in a deployed worker. - Added internal `MxAccessStaSession(Func<MxAccessEventQueue, IAlarmCommandHandler>?)` constructor that builds all defaults but accepts a factory. - Added public `MxAccessStaSession(StaRuntime, factory, eventQueue, alarmFactory?)` 4-arg overload to complete the constructor chain. Gap 2 — WnWrapAlarmConsumer now disables its internal threadpool Timer (pollIntervalMilliseconds=0 in the default constructor). MxAccessStaSession starts a `RunAlarmPollLoopAsync` background task that sleeps off-STA then calls `staRuntime.InvokeAsync(() => handler.PollOnce())` at 500ms intervals. This satisfies the ThreadingModel=Apartment requirement of wwAlarmConsumerClass: every GetXmlCurrentAlarms2 call now runs on the worker's STA. - Added `PollOnce()` to `IMxAccessAlarmConsumer`, `AlarmDispatcher`, `IAlarmCommandHandler`, and `AlarmCommandHandler`. - Poll loop cancelled and awaited before alarm handler disposal in both ShutdownGracefullyAsync and Dispose. Tests: 4 new tests in MxAccessStaSessionTests verify that - SubscribeAlarms reaches the handler when the factory is wired (Gap 1) - SubscribeAlarms returns InvalidRequest without a factory (regression guard) - PollOnce is called on the STA thread within 3s (Gap 2) - The poll loop stops after Dispose (Gap 2 lifecycle) All fake IMxAccessAlarmConsumer / IAlarmCommandHandler test implementations updated with no-op PollOnce() to satisfy the new interface member. Worker tests: 199 passed / 1 pre-existing failure / 4 skipped (was 195/1/4). Server tests: 308 passed / 0 failures (unchanged). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Resolves Client.Rust-001 through Client.Rust-011. Build/test/clippy gate (Client.Rust-001/002/003): - options.rs: doc comments on with_max_grpc_message_bytes / max_grpc_message_bytes (#![warn(missing_docs)]) - session.rs: rename BulkReplyKind variants to drop the shared `Bulk` suffix (clippy::enum_variant_names) - galaxy.rs: deref instead of clone on Option<Timestamp> (clippy::clone_on_copy — an extra violation the gate also hit) - mxgw-cli: assert version_json against GATEWAY/WORKER_PROTOCOL_VERSION constants instead of the stale literal 2 `cargo clippy --workspace --all-targets -- -D warnings` now passes. Correctness / error handling: - version.rs: CLIENT_VERSION = env!("CARGO_PKG_VERSION") (Client.Rust-004) - session.rs: register/add_item/add_item2 handle extractors and bulk_results now return Err(Error::MalformedReply) instead of a silent 0 / empty vec on a shapeless OK reply (Client.Rust-005/006) - error.rs: new Error::Unavailable classifies Code::Unavailable / ResourceExhausted as transient (Client.Rust-010) - session.rs: per-call unique correlation ids via an atomic counter (Client.Rust-011) Other: - value.rs: MxValue/MxArrayValue compute the projection on demand instead of caching it, so a wire-only value pays no projection cost (Client.Rust-008) - RustClientDesign.md: correct the crate layout, drop the unused `tracing` dependency (Client.Rust-007) - client_behavior.rs: tests for the bulk-size cap, a mid-stream status fault, and the unreadable-CA-file path (Client.Rust-009) cargo fmt / test --workspace (27 tests) / clippy all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Contracts-001: docs/Grpc.md still described "four MxAccessGateway RPCs" — updated to the actual six (adding AcknowledgeAlarm and QueryActiveAlarms to the handler and validation-rule sections). Contracts-003 (Won't Fix): the finding is factually wrong — the <Protobuf> item for mxaccess_worker.proto already sets ProtoRoot="Protos"; all three items are consistent (confirmed back to the reviewed commit). Contracts-004: corrected the stale GatewayContractInfo XML summary ("before generated protobuf contracts are introduced"). Contracts-005: no proto field/enum value was ever removed, so no reserved ranges were invented. Added a wire-compatibility policy comment to all three .proto files instructing future editors to reserve removed numbers. Contracts-006: documented MxStatusProxy.success — it mirrors the COM MXSTATUS_PROXY numeric success member, is not a boolean, and clients should branch on category. Contracts-007: added 13 round-trip tests covering galaxy_repository.proto messages, bulk-subscribe payloads, and raw-value/IPC worker bodies. Contracts-008: WorkerAlarmRpcDispatcher never assigns AcknowledgeAlarmReply. status, so the old "native status" proto comment was misleading. Corrected the hresult/status proto comments and documented the worker native_status → public reply mapping in AlarmClientDiscovery.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.