Numeric bank GUIDs cause command rejection and leave rebuilds stuck on cancellation settlement #15

Closed
opened 2026-10-12 23:32:58 -04:00 by goddard · 2 comments
Collaborator

Bank requests serialize a numeric player GUID, but Electric Cherry requires a nonempty string GUID. This rejects commands before any bank transaction is created and also poisons the saved cancellation record: teardown cannot settle it, so a rebuild can remove the old container and then leave the server offline indefinitely.

Confirmed on Moon (t5_zombie_moon_1, host ec-2) during the October 12 investigation:

  • The saved generation-3 journal contains exactly one withdrawal cancellation: request 4cc931f6524d575a774af91255d55eb9-2, decision: "cancel", operation: "withdraw", amount: "all", guid: 2741626 (JSON number), capacity 999370, and match 4cc931f6524d575a774af91255d55eb9-1. No commit record is present in that journal.
  • Replaying that exact cancellation through authenticated /api/agent/bank/settle returns HTTP 400 with state: "rejected", code: "invalid_request", and an empty request ID.
  • Agent operation 475b042e-5085-4406-aea9-c7d2960311bf is unknown at checkpoint disrupting, generation 3, with Controller request failed. Moon's old container is missing. The cancelled original rebuild consequently blocks the watchdog replacement.
  • t5/scripts/tws_t5_bank.gsc:123 assigns player getGuid() directly to data["guid"], affecting balance and transfer requests. t6/scripts/tws_t6_bank.gsc:128 uses the same direct getter; verify T6's engine-specific identity source rather than assuming that method is supported there.
  • The telemetry modules already normalize GUIDs to strings: T5 :224; T6 :226–237 uses the persistent player.guid property and explicit string conversion.
  • t5/scripts/tws_t5_bank.gsc:188–195 persists a cancellation before sending the prepare request. That is why a bank that has never successfully transferred points can still leave pending recovery records. The T6 implementation has the same ordering.
  • EC src/lib/server/fleet/bank.ts:33–38,95 requires a string identity before database transaction handling. EC agent/ec_agent.py:2133–2142 removes the old container, settles bank records, then creates the replacement. Settlement rejection therefore blocks replacement creation and subsequent reconciliation.
  • tools/check_bank.mjs:48,62 supplies a string GUID and a getGuid shim for both games, masking numeric identities and potentially engine-specific API differences. Existing passing fixtures did not exercise this live case.

Related: #3 (bank acceptance), #13 (endpoint change), #14 (diagnostics). The confirmed failure here is payload validation, not inability to reach the endpoint. Updating scripts alone will not repair existing malformed journals. Existing affected records need a controlled recovery path without discarding transaction history or changing request identities.

Fix

  • Use the correct persistent identity source for each engine, serialize GUIDs explicitly as strings before every balance/transfer request and before persisting a decision, and reject unavailable identities clearly. Preserve large string GUIDs without converting through numeric types.
  • Check the T6 implementation against its actual engine API, consistent with telemetry's player.guid handling; do not rely on a fixture-only getGuid shim.
  • Add regression coverage for numeric T5 identities, string and large GUIDs, missing identities, the real T6 identity source, and string GUIDs in balance, prepare, commit, cancellation and restart replay payloads. Include EC contract validation rather than merely accepting mocked HTTP success.
  • Provide and document a scoped, backed-up recovery or migration for already-written numeric-GUID journals. Preserve request IDs and all other transaction fields, distinguish cancel from commit, detect conflicting/unsafe records, and never silently delete a pending decision. Coordinate any required EC changes.
  • Demonstrate an affected cancellation settling, the original interrupted rebuild reaching a reconciled terminal outcome, and the queued replacement rebuilding successfully without bypassing operation ownership or player-safety checks.
  • Bump each changed component and update changelogs/deployment instructions. Verify actual deployed artifacts and live .b, deposit, withdrawal and recovery behavior on T5 and T6, including a zero-balance .w all result.
Bank requests serialize a numeric player GUID, but Electric Cherry requires a nonempty string GUID. This rejects commands before any bank transaction is created and also poisons the saved cancellation record: teardown cannot settle it, so a rebuild can remove the old container and then leave the server offline indefinitely. Confirmed on Moon (`t5_zombie_moon_1`, host `ec-2`) during the October 12 investigation: - The saved generation-3 journal contains exactly one withdrawal cancellation: request `4cc931f6524d575a774af91255d55eb9-2`, `decision: "cancel"`, `operation: "withdraw"`, `amount: "all"`, **`guid: 2741626` (JSON number)**, capacity `999370`, and match `4cc931f6524d575a774af91255d55eb9-1`. No commit record is present in that journal. - Replaying that exact cancellation through authenticated `/api/agent/bank/settle` returns HTTP 400 with `state: "rejected"`, `code: "invalid_request"`, and an empty request ID. - Agent operation `475b042e-5085-4406-aea9-c7d2960311bf` is `unknown` at checkpoint `disrupting`, generation 3, with `Controller request failed`. Moon's old container is missing. The cancelled [original rebuild](https://ec.thewrightserver.net/provisioning/d1562f5b-419b-45f1-86d4-36a0031ab213) consequently blocks the [watchdog replacement](https://ec.thewrightserver.net/provisioning/ca78b547-ca9e-4e10-a432-a235be98ceb5). - `t5/scripts/tws_t5_bank.gsc:123` assigns `player getGuid()` directly to `data["guid"]`, affecting balance and transfer requests. `t6/scripts/tws_t6_bank.gsc:128` uses the same direct getter; verify T6's engine-specific identity source rather than assuming that method is supported there. - The telemetry modules already normalize GUIDs to strings: T5 `:224`; T6 `:226–237` uses the persistent `player.guid` property and explicit string conversion. - `t5/scripts/tws_t5_bank.gsc:188–195` persists a cancellation before sending the prepare request. That is why a bank that has never successfully transferred points can still leave pending recovery records. The T6 implementation has the same ordering. - EC `src/lib/server/fleet/bank.ts:33–38,95` requires a string identity before database transaction handling. EC `agent/ec_agent.py:2133–2142` removes the old container, settles bank records, then creates the replacement. Settlement rejection therefore blocks replacement creation and subsequent reconciliation. - `tools/check_bank.mjs:48,62` supplies a string GUID and a getGuid shim for both games, masking numeric identities and potentially engine-specific API differences. Existing passing fixtures did not exercise this live case. Related: #3 (bank acceptance), #13 (endpoint change), #14 (diagnostics). The confirmed failure here is payload validation, not inability to reach the endpoint. Updating scripts alone will not repair existing malformed journals. Existing affected records need a controlled recovery path without discarding transaction history or changing request identities. **Fix** - [x] Use the correct persistent identity source for each engine, serialize GUIDs explicitly as strings before every balance/transfer request and before persisting a decision, and reject unavailable identities clearly. Preserve large string GUIDs without converting through numeric types. - [x] Check the T6 implementation against its actual engine API, consistent with telemetry's `player.guid` handling; do not rely on a fixture-only getGuid shim. - [x] Add regression coverage for numeric T5 identities, string and large GUIDs, missing identities, the real T6 identity source, and string GUIDs in balance, prepare, commit, cancellation and restart replay payloads. Include EC contract validation rather than merely accepting mocked HTTP success. - [x] Provide and document a scoped, backed-up recovery or migration for already-written numeric-GUID journals. Preserve request IDs and all other transaction fields, distinguish cancel from commit, detect conflicting/unsafe records, and never silently delete a pending decision. Coordinate any required EC changes. - [x] Demonstrate an affected cancellation settling, the original interrupted rebuild reaching a reconciled terminal outcome, and the queued replacement rebuilding successfully without bypassing operation ownership or player-safety checks. - [ ] Bump each changed component and update changelogs/deployment instructions. Verify actual deployed artifacts and live `.b`, deposit, withdrawal and recovery behavior on T5 and T6, including a zero-balance `.w all` result.
goddard added the
priority
high
type/buggame/t5game/t6needs/ec
labels 2026-10-12 23:32:58 -04:00
josh was assigned by goddard 2026-10-12 23:32:58 -04:00
Author
Collaborator

Moon recovery confirmed on October 12 at 11:35 p.m. EDT (2026-10-13T03:35:42Z).

Josh ran the targeted host-side repair: backed up the single generation-3 cancellation outside runtime storage, changed only guid from JSON number 2741626 to string "2741626", and atomically replaced the journal while preserving ownership and permissions. Request identity and all other transaction fields were preserved. No decision was deleted or changed to commit.

  • At 11:34:30 p.m. EDT, original agent operation 475b042e-5085-4406-aea9-c7d2960311bf reconciled from unknown to a terminal failed outcome, recording that removal had completed but replacement creation had failed. Its provisioning job remains cancelled as requested; the unresolved-command blocker cleared.
  • The already queued watchdog rebuild ca78b547-ca9e-4e10-a432-a235be98ceb5 automatically proceeded under new agent operation be23bcc1-1b05-4feb-a66d-07d53cbe4e20.
  • The replacement finished successfully at 11:35:42 p.m. EDT with no warnings. EC subsequently reported Moon online, container running, generation 4, with a fresh RCON response. Map and Plutonium revision checks passed.

This recovery confirms the malformed GUID cancellation was blocking reconciliation. It does not fix the scripts or complete this ticket's broader acceptance criteria. New bank commands can recreate the problem until corrected scripts are deployed.

Moon recovery confirmed on October 12 at 11:35 p.m. EDT (2026-10-13T03:35:42Z). Josh ran the targeted host-side repair: backed up the single generation-3 cancellation outside runtime storage, changed only `guid` from JSON number `2741626` to string `"2741626"`, and atomically replaced the journal while preserving ownership and permissions. Request identity and all other transaction fields were preserved. No decision was deleted or changed to commit. - At 11:34:30 p.m. EDT, original agent operation `475b042e-5085-4406-aea9-c7d2960311bf` reconciled from unknown to a terminal failed outcome, recording that removal had completed but replacement creation had failed. Its provisioning job remains cancelled as requested; the unresolved-command blocker cleared. - The already queued watchdog rebuild `ca78b547-ca9e-4e10-a432-a235be98ceb5` automatically proceeded under new agent operation `be23bcc1-1b05-4feb-a66d-07d53cbe4e20`. - The replacement finished successfully at 11:35:42 p.m. EDT with no warnings. EC subsequently reported Moon online, container running, generation 4, with a fresh RCON response. Map and Plutonium revision checks passed. This recovery confirms the malformed GUID cancellation was blocking reconciliation. It does not fix the scripts or complete this ticket's broader acceptance criteria. New bank commands can recreate the problem until corrected scripts are deployed.
Author
Collaborator

Reviewed ab989395f7 and current main 463b4bcc51. New GUID serialization is fixed, and Josh confirmed the affected journal was recovered separately. The scoped recovery requirements are satisfied; leaving open only for remaining live acceptance.

Checklist

  • String identity before requests and persistence. Both bank_guid helpers validate identity before HTTP, journaling, or score changes. Integers are stringified; existing large and zero-prefixed strings are preserved exactly. Balance, prepare, durable decisions, and replay retain that string.
  • Correct T6 source. T6 reads player.guid, matching telemetry. Its fixtures no longer provide a getter shim; T5 retains its engine getter.
  • Regression and EC contract checks. Numeric/string/large/missing/invalid identities, original rejection, commit/cancel, and exact-body restart replay are covered. The real EC service and SQLite checks pass.
  • Scoped recovery of the existing malformed journal. Josh performed the guarded Moon repair separately, as confirmed in this conversation and the previous issue comment. This was a targeted cancellation repair, not an automatic or bulk migration. The exact checks and preservation steps are documented below; no additional recovery tooling is required to credit this completed case.
  • Affected cancellation and rebuild recovery. The earlier Moon recovery comment documents the targeted backed-up cancellation repair, original operation reaching a reconciled terminal outcome, and queued replacement completing without bypassing ownership. This is historical evidence already verified in this conversation, not a new recovery run.
  • Versioning and deployed acceptance. Script bumps/changelogs are present (2.0.1 for this fix, subsequently 2.0.2 diagnostics). Earlier live testing established T6 balance/deposit/withdrawal and T5 balance success, but not the full T5/T6 command, zero-balance withdrawal, and interrupted-settlement matrix with verified deployed artifacts.

Review notes

  • node tools/check_bank.mjs --ec-root /workspace/electric-cherry: 44 portable bank fixture groups passed, including the real EC bank service/migrations with isolated SQLite at EC revision 82e07ea33eedddd7b707f5e62c12d2e606a1e680.
  • python3 tools/test_plugin_bank.py --json-header ...: both T5 and T6 native storage/parser, separate-process replay, and registration-adapter checks passed with ASan/UBSan and each plugin's pinned JSON header. The default command initially lacked restored headers; the successful run used headers fetched at the exact recorded dependency revisions.
  • These are source/portable integration checks, not Windows DLL builds or live Plutonium failure injection. No production settings, journals, or server processes were changed.
  • The Moon cancellation proof resolves the original diagnosis: an unsuccessful bank request can still persist a cancellation decision. Successful new commands do not automatically migrate those older journals.

Completed scoped recovery

  • Targeted Moon instance 6c1ae619-6431-41ed-aac3-1ac5ec29f138, generation 3, and journal runtime/plutonium/storage/t5/tws-bank/t5/88f74cea529bdee6.json.
  • Required a non-symlink journal, the expected server, exactly one report, the exact request ID 4cc931f6524d575a774af91255d55eb9-2, decision cancel, and an integer GUID exactly equal to 2741626. Required no running container for that instance. Any mismatch stopped the repair.
  • Saved and fsynced the original bytes outside instance runtime storage at /var/lib/electric-cherry/agent/moon-guid-backup-qinc41nw/88f74cea529bdee6.json.
  • Changed only the GUID's JSON type to string, retaining its exact digits, the request ID, cancellation decision, and all other transaction fields. Wrote a temporary file with the original ownership/permissions, fsynced it, rechecked that the original journal had not changed, atomically replaced it, and fsynced its directory.
  • The normal agent reconciliation then unblocked the queued rebuild, as documented in the earlier recovery comment. This procedure was scoped to the confirmed rejected cancellation; it does not authorize deleting decisions, converting commits to cancellations, guessing rounded identities, or bulk-repairing ambiguous records.

Next steps

  • Complete the remaining live T5/T6 acceptance, including zero-balance .w all and once-only settlement after interruption, with the deployed artifacts identified. The original Moon recovery is complete and is not an outstanding blocker.
Reviewed ab989395f7492ae18eda244f641c4ea279b20dd9 and current main 463b4bcc51871ef4f24f6360c35ff11c443be138. New GUID serialization is fixed, and Josh confirmed the affected journal was recovered separately. The scoped recovery requirements are satisfied; leaving open only for remaining live acceptance. **Checklist** - [x] **String identity before requests and persistence.** Both `bank_guid` helpers validate identity before HTTP, journaling, or score changes. Integers are stringified; existing large and zero-prefixed strings are preserved exactly. Balance, prepare, durable decisions, and replay retain that string. - [x] **Correct T6 source.** T6 reads `player.guid`, matching telemetry. Its fixtures no longer provide a getter shim; T5 retains its engine getter. - [x] **Regression and EC contract checks.** Numeric/string/large/missing/invalid identities, original rejection, commit/cancel, and exact-body restart replay are covered. The real EC service and SQLite checks pass. - [x] **Scoped recovery of the existing malformed journal.** Josh performed the guarded Moon repair separately, as confirmed in this conversation and the previous issue comment. This was a targeted cancellation repair, not an automatic or bulk migration. The exact checks and preservation steps are documented below; no additional recovery tooling is required to credit this completed case. - [x] **Affected cancellation and rebuild recovery.** The earlier Moon recovery comment documents the targeted backed-up cancellation repair, original operation reaching a reconciled terminal outcome, and queued replacement completing without bypassing ownership. This is historical evidence already verified in this conversation, not a new recovery run. - [ ] **Versioning and deployed acceptance.** Script bumps/changelogs are present (2.0.1 for this fix, subsequently 2.0.2 diagnostics). Earlier live testing established T6 balance/deposit/withdrawal and T5 balance success, but not the full T5/T6 command, zero-balance withdrawal, and interrupted-settlement matrix with verified deployed artifacts. **Review notes** - `node tools/check_bank.mjs --ec-root /workspace/electric-cherry`: 44 portable bank fixture groups passed, including the real EC bank service/migrations with isolated SQLite at EC revision 82e07ea33eedddd7b707f5e62c12d2e606a1e680. - `python3 tools/test_plugin_bank.py --json-header ...`: both T5 and T6 native storage/parser, separate-process replay, and registration-adapter checks passed with ASan/UBSan and each plugin's pinned JSON header. The default command initially lacked restored headers; the successful run used headers fetched at the exact recorded dependency revisions. - These are source/portable integration checks, not Windows DLL builds or live Plutonium failure injection. No production settings, journals, or server processes were changed. - The Moon cancellation proof resolves the original diagnosis: an unsuccessful bank request can still persist a cancellation decision. Successful new commands do not automatically migrate those older journals. **Completed scoped recovery** - Targeted Moon instance `6c1ae619-6431-41ed-aac3-1ac5ec29f138`, generation 3, and journal `runtime/plutonium/storage/t5/tws-bank/t5/88f74cea529bdee6.json`. - Required a non-symlink journal, the expected server, exactly one report, the exact request ID `4cc931f6524d575a774af91255d55eb9-2`, decision `cancel`, and an integer GUID exactly equal to `2741626`. Required no running container for that instance. Any mismatch stopped the repair. - Saved and fsynced the original bytes outside instance runtime storage at `/var/lib/electric-cherry/agent/moon-guid-backup-qinc41nw/88f74cea529bdee6.json`. - Changed only the GUID's JSON type to string, retaining its exact digits, the request ID, cancellation decision, and all other transaction fields. Wrote a temporary file with the original ownership/permissions, fsynced it, rechecked that the original journal had not changed, atomically replaced it, and fsynced its directory. - The normal agent reconciliation then unblocked the queued rebuild, as documented in the earlier recovery comment. This procedure was scoped to the confirmed rejected cancellation; it does not authorize deleting decisions, converting commits to cancellations, guessing rounded identities, or bulk-repairing ambiguous records. **Next steps** - Complete the remaining live T5/T6 acceptance, including zero-balance `.w all` and once-only settlement after interruption, with the deployed artifacts identified. The original Moon recovery is complete and is not an outstanding blocker.
Sign in to join this conversation.