Make trades atomic and build portal trade investigations
Goal Current delivery: build the approved staff trade-investigation system into PRK.PlayerPortal, with its persistent icon-and-label tools sidebar, searchable exchange table and contextual detail inspector. Capture real trade evidence from PlayfieldEngine. The original cross-engine structured logging work remains recorded below but is deferred from this portal delivery. Authorized implementation scope - Refactor trade completion so all affected player snapshots and the immutable completed-trade audit record persist in one SQL transaction. Remove independent per-player emergency saves from the trade path. - Coordinate with periodic/logout saves, preserve existing transfer/stack/container/vendor behavior, and handle rollback/ambiguous commit without allowing a later save or replay to persist one side alone. - Deliver the approved portal investigation layout with a reusable role-aware tools sidebar, per-server authorization, bounded database filtering/sorting/pagination, exchange details and safe export. No production deployment without validation. - Verify second-player/audit-write failures roll back all database changes; successful records match persisted transfers; replay is idempotent; cancellation/failure never masquerades as completed. - Broader Login/Chat logging changes, Grafana provisioning and live Loki validation below are deferred, not gates for this delivery. Original logging scope and constraints (deferred) - Inspect the existing shared logging infrastructure and Grafana/Loki ingestion before choosing changes; reuse common engine configuration and abstractions rather than introducing parallel logging pipelines. - Define consistent event names, typed fields and correlation context across Login, Chat and Playfield engines. - Capture player trade outcomes with participant identities, exchanged items and credits, and transaction correlation; distinguish completed trades from failed or cancelled attempts. - Preserve useful human-readable messages while retaining structured fields through collection and ingestion. - Keep high-cardinality player/transaction identifiers out of Loki index labels; avoid secrets and unnecessary personal data. Acceptance criteria - Structured fields survive end-to-end ingestion and can be filtered in Grafana without parsing prose. - Player trades support staff investigation of player claims: look up either participant and a time range, reconstruct both directions of the exchange and distinguish offers from actual transfers and confirmed persistence. - Trade evidence includes UTC timestamp, stable trade ID, participant character IDs and name snapshots, engine/playfield context, and credits/currency transferred in each direction. Item snapshots include instance ID, template IDs, name, quality level, stack quantity and claim-relevant instance attributes; transferred containers include their contents. Evidence must not depend on current ownership, mutable templates or later item state. - Staff can inspect the complete exchanged-item list without silent truncation; bounded multi-record payloads, if needed, share the trade ID and expose completeness. Failed, cancelled and partially persisted trades are clearly distinguishable from confirmed completed exchanges. - Other gameplay events can adopt the same shared event conventions. - Tests verify event fields, trade outcome semantics and safe serialization; ingestion configuration and representative Grafana queries are validated. - PF, Login and Chat share logging configuration and event conventions while preserving existing Sentry, category filters and useful local console output. - Login authentication/throttling, character selection/handoff, and Chat authentication/session/channel/routing outcomes expose stable event names, outcome/reason fields and safe identity context. Private message bodies, credentials, cookies and raw packets are excluded by default. - Backend failure does not block gameplay or authentication; export buffering and shutdown flush are bounded, and duplicate export paths are avoided. Repository evidence and implementation direction - All three Program.cs files already configure Microsoft.Extensions.Logging with optional OpenTelemetry OTLP export and Sentry. Consolidate duplicated registration, including Sentry/no-Sentry branches, into shared infrastructure; retain ILogger rather than introducing a competing logging stack. - Deployment configuration already targets Loki /otlp. observability/loki.yaml enables structured metadata; align both checked-in Loki configurations and validate deployed field mapping before assuming the production ingestion cause. - TradeWindow already has a stable trade ID, lifecycle hooks and extensive regression tests. Replace packed LogCtx strings with independent typed attributes; capture bounded immutable item/credit snapshots rather than serializing live entities. Complete OK currently precedes CommitAsync completion: distinguish transferred, persisted, notification failure, partial/uncertain persistence, recovery failure and cancellation without claiming an atomic cross-player transaction. - LoggingHelper currently sends rich context to Sentry but only the message/exception to ILogger. Extend the existing helper with safe structured context rather than creating a second error pipeline. - Migrate interpolation and Console.WriteLine operational paths in the three engines in stages, retaining already structured templates. Share event/field conventions and lightweight helpers, not one giant event payload model. Reuse existing trade, login, chat and logging test suites. Ordered delivery 1. Establish an end-to-end ingestion repro and event/field/privacy contract. 2. Consolidate shared logger registration, resource identity, filters and safe context; verify Sentry enabled/disabled and local/export modes. 3. Implement structured trade lifecycle and truthful persistence outcomes. 4. Migrate Login and Chat lifecycle/security/routing events and remaining in-scope message-only operational logs; preserve PFE performance logs. 5. Provide Grafana views and LogQL queries for trade history, player/session timelines and errors across engines, using bounded labels and non-indexed player/trade fields. 6. Validate automated regressions, real Loki field round trips, outage behavior and representative-load overhead; canary staging before production rollout. Staff investigation workflow - Supplied context calls for readable, filterable exchange history: search character names/IDs and time range, identify counterparties, distinguish player trades from vendor purchases/sales, and inspect items and credits in each direction. - Include participant/vendor display names and stable identifiers in event snapshots; support filters for exchange kind and outcome. The approved portal interface is now the primary delivery, reusing existing Razor Pages, staff authorization, per-server access and game-database connection patterns. Include server-side bounded filtering/pagination/sorting, staff-only detail/export, immutable item/credit snapshots and truthful save-outcome handling. Keep read-only investigation access separate from account/player mutation permissions. - This supports investigation of alleged illicit trading and player claims; transfers alone do not establish real-money payment or shared account ownership. Missing retained logs are not proof that a trade did not occur. - Completed exchange evidence now uses the game database as its source of truth. Loki remains supplementary diagnostics; Grafana and cross-engine logging cleanup are not prerequisites for this portal delivery. Pending product decisions - Resolved: the user authorized atomic player persistence and a durable completed-trade ledger in the game database. Cancellation/failure records and post-commit diagnostics are best effort. Gameplay service effects execute after the financial/inventory transaction and have attempted/unconfirmed or failed diagnostic status; a void API return is not positive application evidence; no automatic effect replay is promised. - Whether chat message bodies should be retained at all, and if so which message classes, access controls and retention policy apply. Default proposal is metadata-only. Preparation Worktree: /home/mathias/Projects/worktrees/engines-structured-logging Branch: feature/structured-engine-logging Base: origin/develop at 3068eb7d9. Approved interactive mockup is at /tmp/prk-trade-investigation-mockup. User explicitly authorized refactoring atomic trade persistence alongside the portal audit UI. Investigation confirmed each player repository Update opens its own transaction and mutates cached persistence baselines; coordination must cover these baselines and commit semaphores, not just Task.WhenAll. Local dotnet SDK is absent; existing SDK Docker images are available for isolated builds/tests. Implementation in progress in the prepared worktree. Verified foundation milestone - Added UpdateTrade to the player repository, reusing UpdateCore with one shared SQL connection/transaction, sorted participant locks, deferred baseline advancement, full snapshot restoration and rollback of generated IDs. Completed audit insert shares the same transaction. Commit acknowledgement failures have a distinct uncertain exception, not a false rollback promise. - Added additive game migration 0076-TradeAudits.sql, structured snapshot models, staff-only Razor history/detail/export, per-server reads, 31-day date bound, 50-record pages, validated sorting, no-store responses and encoded item/bag evidence. Shared icon/label navigation is enabled for admin pages. Trade access requires fresh administrator/management/developer roles under the configured OIDC client; unrelated-client/unscoped roles and bug_hunter do not grant access. - 83 targeted tests pass with real disposable SQL, including second-player failure, audit failure, item ownership rollback, duplicate trade IDs, concurrent ordinary-save exclusion, lost commit acknowledgement, database filtering, authorization and actual Razor rendering/export. PFE and portal builds pass. Browser verified actual Razor UI at desktop/mobile widths, filtering/sorting/pagination, expanded bag IDs, uncertain warning, close/Escape/back behavior and invalid dates; fixed small metadata typography after visual review. Export JSON contents verified in HTTP tests. One unidentified browser resource404 remains nonblocking. - Local fictional SQL-backed portal preview: http://127.0.0.1:8890/preview/sign-in . Its temporary login exists ONLY in /tmp/prk-audit-preview, not production source. Live integration implemented and locally verified - Foundation committed as c228fe8cd (feature: Add trade audit persistence and portal investigation UI). Live integration committed as 9c6112022 (fix: Persist trades atomically with portal audit evidence); worktree clean. No deployment performed. The SSH-agent blocker is resolved: the user requested a retry and the normal push succeeded, establishing origin/feature/structured-engine-logging as upstream. Published head is 4936df3de; the latest requested review fixes are local and uncommitted. PR #2727 โ https://git.project-rk.com/prk/engines/pulls/2727 โ feature/structured-engine-logging into develop in prk/engines. PR documents the known allocation-test flake and outstanding real-client staging release gate. - Live completion now acquires ordered save leases before character locks, uses detached persistence snapshots, and writes every affected player plus immutable completed evidence atomically. Removed independent trade CommitAsync/emergency saves. Definite failures restore inventory/bag ownership, stack quantities and currency before returning offers; uncertain commits fence saves and queued inbound work and disconnect old sessions. - Own staged escrow suppresses ordinary snapshots/cadence saves; empty windows do not suppress cadence. Window reservations use compare-exchange and ordered begin locks to avoid overlapping counterparties. Completion revalidates range/instance. UNIQUE escrow remains owned for acquisition checks; timed/effect/container items are not incorrectly merged. Unsupported nested bags are rejected. Cancelled items can survive overflow save/reload; inventory-audit geometry and unsupported-page fixtures now match that support. - Vendor delivery messages and service effects run after the shared transaction. Completed evidence is mandatory/direct; a bounded 64-entry worker records non-completed outcomes and delivery diagnostics without blocking gameplay. Portal timelines distinguish financial persistence, client notification failure and service execution in memory; missing service events remain explicitly unconfirmed. - Added migrations 0077 (events) and 0078 (decoded searchable item projection with legacy nested-name backfill). Apostrophes, quotes and Unicode names are searchable independently of JSON encoding. Export includes UTC timestamps, server tag, schema version, trade and events. CI's existing engine job now has one disposable SQL service so SQL regressions run rather than skip; workflow parsed and deployment configuration tests passed locally, remote CI not run. - Final full sharded unit suite: 5,404 passed, 73 skipped. One repeat hit the unmodified DoorIndex_FarDoorsAreNotCandidatesAndReadsAllocateNothing assertion (6,184 measured bytes versus zero); three isolated runs and the final complete suite passed. Root cause was not established and no door code/test was changed. Engine and portal builds have zero warnings/errors. Real-SQL tests cover actual trade handlers, second-player/audit failure, ownership rollback, zero-credit gifts with clean baselines, duplicate IDs and commit ambiguity. Strengthened repository-lock test was mutation-verified: removing participant locks fails with 1 expected versus 4 actual DB contexts, and the real locks were restored. HTTP tests verify authorization, encoding, no-store export with events/server/UTC; Chrome verifies actual Razor desktop/mobile search, sorting, pagination, bag details, uncertain/service-failure timeline and return navigation. Two static findings from the foundation review (JSON search and weak lock assertion) were fixed. Full branch static review covered all 51 changed diffs and found one client-placement contract issue. Fixed in bb1f5268e: returned escrow uses the first free main-inventory slot and sends best-effort overflow updates without depending on socket success. Four new/strengthened regressions failed before the fix and passed afterward; follow-up static review found no actionable findings. Latest develop (30bf94443) merged cleanly in 4936df3de, including updated role-refresh guidance. Merged engine/portal builds pass; 154 targeted tests pass. Merged full suite reported 5,423 passed, 73 skipped and the known intermittent unmodified door-allocation assertion, independently tracked in PR #2713. Review findings fixed locally (published PR head remains 4936df3de) - HIGH resolved locally: shared AcquireGameplayLockAsync rechecks fencing after acquisition and releases rejected leases. Current gameplay handler/command locks use it; paired trade locks explicitly recheck both characters. Inventory mutation boundaries also reject fenced actors. Real ContainerAddItemMessageHandler tests reproduce the previously failing shared-loot race, include a successful unfenced control and raw-lock cleanup, and cover an actual concurrent trade acknowledgement failure while loot waits. - MEDIUM resolved locally: normal void-effect returns now emit service_effect_attempted, never applied. New writes reject the old applied kind; portal labels historical applied events as unconfirmed and retains the warning. A real GiveItem -> FunctionManager -> CastNanoSpell missing-nano rejection reproduced false confirmation before the fix. SQL/HTTP tests cover attempted, failed and historical-applied rendering/export; Chrome verified the attempted/unconfirmed warning and open timeline at desktop/mobile sizes. Positive effect confirmation is intentionally not claimed without a result-bearing effect API. - MEDIUM resolved locally: the zone-in follow-up after FCM announces nonempty overflow before its per-instance temporary state, directly to the session without requiring a current cell. Overflow mission keys are replayed only after QFU alongside main-inventory keys. Six persistence/reload/message cases cover plain and temporary items, weapons and mission keys, including timeexist, overflow placement and quest ordering; empty overflow remains silent. - Validation: 30 focused regressions pass; defects were reproduced red before their fixes. Engine and portal builds pass with zero warnings/errors. Full sharded suite: 5,438 passed, 73 skipped, one unmodified DoorIndex zero-allocation failure (6,328 bytes vs zero), the known #2713 flake. Chrome verified the corrected warning/desktop-mobile behavior; browser download contents were not inspected, while HTTP tests verified exports. The always-on MSSQL CI service was left unchanged. No commits, push, PR mutation or deployment performed for this fix request; published PR still needs these changes. Real-client staging remains outstanding. Remaining release validation - Smoke-test the new PFE with a real client in staging: player trades both directions, credits-only, bags/merged and timed stacks, vendor buys/sells/non-credit currencies, cancellation/zone/logout, reconnect persistence and service effects. This has not been run and production is not approved. - Apply additive game migrations 0076โ0078 before the portal reader; update all PFEs before relying on audit coverage. No automatic historical log reconstruction or audit retention/deletion policy is enabled. README documents contracts, limits, SQL tests, rollout and rollback.
Checklist
6 of 12 complete- Verify a baseline ILogger-to-Loki round trip and define stable event names, typed fields, correlation and privacy/label rules.
- Consolidate logging setup and safe context across PF, Login and Chat; verify filters, resource identity, console output and Sentry on/off behavior.
- Implement and regression-test structured player trade lifecycle, item/credit snapshots and accurate persistence, cancellation and failure outcomes.
- Migrate Login and Chat lifecycle/security/routing events and in-scope message-only operational logs; verify correlation, redaction and existing behavior.
- Validate Loki metadata/label mapping and provision Grafana trade history, cross-engine player/session and error queries/views.
- Pass relevant engine builds/tests, real Loki round-trip and outage/flush checks, and staging load/canary validation; document rollout and rollback.
- Verify atomic repository writes with real SQL: second participant/audit failures roll back both players and items, duplicate IDs cannot reapply, and ordinary saves wait for trade transaction.
- Build and verify staff-only portal audit read UI, tools sidebar, bounded SQL filtering/sorting/pagination, encoded snapshots and no-store export using real Razor HTTP and desktop/mobile browser tests.
- Pass real-client staging smoke tests for atomic trades, reconnect persistence, bags/stacks, shop currencies/services and cancellation/zone/logout before production rollout.
- Replay persisted overflow inventory and temporary item state after zone-in initialization, preserving mission-key quest ordering; verify reload messages.
- Verify already-dispatched gameplay handlers recheck fencing after acquiring locks and cannot consume shared loot or mutate quarantined inventory.
- Record non-throwing service execution as attempted/unconfirmed unless application is explicitly confirmed; cover a real effect rejection and portal warnings.
- Area
- Project Rubi-Ka
- Project
- engines
- Kind
- Feature
- Task ID
tsk_5vmwwawx7u- Updated