Skip to content

CI: fix every failure on development since #137, and un-quarantine its two tests - #139

Open
Cadacious wants to merge 4 commits into
InfiniteRasa:developmentfrom
SandboxServers:ci/fix-post-merge-failures
Open

Cadacious wants to merge 4 commits into
InfiniteRasa:developmentfrom
SandboxServers:ci/fix-post-merge-failures

Conversation

@Cadacious

Copy link
Copy Markdown
Contributor

Every CI run on development since #137 merged has failed. This fixes each failure, including the two tests #137 quarantined, so the suite runs with nothing skipped.

Failures fixed

Test Failed on Cause Fix
AssigningPlayerRefreshesTheWeaponDrawerAfterSelectingTheControlledActor every run since #136 #136 moved SetControlledActorId after the initial mission state (needed for mission tracking), but the weapon-tray refresh stayed in place, so it went out before the controlled actor The tray refresh (drawer contents, weapon and ability selection) now goes right after SetControlledActorId. Options and mission state still come first, which keeps #136's ordering.
GenericMissionRuntimeFilesDoNotContainBootcampMissionIdSwitches every run since #138 #138 put a Bootcamp mission id (2005) in a doc comment in MissionApplication.cs, which this guard test forbids The comment names the mission instead of its id
ConcurrentDeadlineEvaluationCommitsOneCompletion (quarantined in #137) intermittently A real race: SceneApplication.Tick took due timers from SceneDueQueue outside _dispatchGate, and Resume re-attached runs from their rows outside it, while commits from client threads run under the gate. Overlapping evaluations could corrupt the queue, or re-attach a run one revision behind a commit, so every commit was rejected ("Concurrent scene revision") and the deadline never completed SceneDueQueue locks its own state. Tick takes due work and submits it under the gate. Resume runs under the gate. No new lock-order edges: the gate already covers Commit.
ForeanEscortsCanFollowFromTheCaveExitToTheReclaimedBase (quarantined in #137) intermittently Escorts fight hostiles within 20 m of their owner. At the base they engaged the Thrax, and where the fight left them on the last tick depended on wall-clock creature timers (Environment.TickCount64) The test covers the route, not the fights, so it removes the creatures hostile to the escorts before advancing

Validation

  • Reproduced locally in mcr.microsoft.com/dotnet/sdk:10.0.401 (the SDK version global.json pins), with the work dir and /tmp on tmpfs. All four failures reproduced. Stress loops of the two intermittent tests, before → after:
    • concurrent deadline: 12/500 failed → 0/2000
    • Forean escort: 6/15 failed → 0/40
  • Full suite locally: 9 of 9 CI-equivalent runs green: 16 lanes planned by TestShards.cs, 3,283 tests, 0 failed, 0 skipped.
  • Full CI on GitHub runners (fork validation PRs [fork test] Fix the CI failures since #137 SandboxServers/Rasa.NET#4 and Network architecture #5, same commits): 8 of 8 runs green, each with 3,283 results, 3,283 passed, 0 failed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AxABTHjs6nJrsXVCTnmMo7

Cadacious and others added 4 commits October 5, 2026 07:08
InfiniteRasa#136 moved SetControlledActorId after the initial mission state, so the
retail client loads its MissionTrack options against the current mission
list. The weapon drawer refresh and the weapon and ability selections
stayed where they were, now before the controlled actor, and
AssigningPlayerRefreshesTheWeaponDrawerAfterSelectingTheControlledActor
failed on every run since. They now follow SetControlledActorId: options
and mission state still come first, and the tray still gets its contents
after its actor, then its selection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AxABTHjs6nJrsXVCTnmMo7
InfiniteRasa#138 named the retry mission's id in a doc comment, and
GenericMissionRuntimeFilesDoNotContainBootcampMissionIdSwitches guards
the generic mission runtime against Bootcamp ids. The comment names the
mission instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AxABTHjs6nJrsXVCTnmMo7
SceneApplication.Tick took due timers from SceneDueQueue (a PriorityQueue
and a Dictionary) outside _dispatchGate, and Resume re-attached runs from
their rows outside it too, while commits from client threads run under
it. Two overlapping evaluations could corrupt the queue, or one could
re-attach a run a revision behind the other's commit, so both commits were
rejected ("Concurrent scene revision") and the deadline never completed.

- SceneDueQueue locks its own state.
- Tick takes due work and submits it under the dispatch gate.
- Resume runs under the dispatch gate.

ConcurrentDeadlineEvaluationCommitsOneCompletion is back in the suite:
it failed 12 in 500 iterations under load before, 0 in 2000 after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AxABTHjs6nJrsXVCTnmMo7
An escort engages hostiles within 20 of its owner, so at the reclaimed
base it fought the Thrax, and where that fight left it at the last tick
depended on creature timers that run on Environment.TickCount64. The
test, which is about the route, removes the creatures hostile to the
escorts first, and is back in the suite: it failed 6 in 15 runs before,
0 in 40 after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AxABTHjs6nJrsXVCTnmMo7
Comment on lines -1958 to -1969
// Inventory deltas precede LoginOk. Refresh the tray after its controlled actor
// exists so the initial image does not depend on opening the equipment selector.
client.CallMethod(SysEntity.ClientInventoryManagerId,
new Packets.Inventory.Server.InventoryCreatePacket(
InventoryType.WeaponDrawerInventory, player.Inventory.WeaponDrawer.ToList(),
player.Inventory.WeaponDrawer.Count));
client.CallMethod(player.EntityId, new WeaponDrawerSlotPacket(player.ActiveWeapon, false));

// The armed ability too, with its loadout page: not requested, so the client takes it
// as its requested slot as well.
client.CallMethod(player.EntityId, new AbilityDrawerSlotPacket(player.CurrentAbilityDrawer, false));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ordering change here, what functionally wise does it do?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants