[Bug] ACS Defend destroyed fleet stays in host widget after hold expires #1321

[piciolo]

Description

When an ACS Defend fleet was completely destroyed in battle during its hold time, the fleet mission would disappear from the host's widget immediately after the battle — but then reappear and stay stuck in the widget once the original time_arrival (hold end time) was reached, remaining there for another full time_holding duration before finally clearing.

Root Cause

getActiveFleetMissionsForCurrentPlayer() contained a legacy orWhere fallback that re-included ACS Defend outbound missions with processed=1 in the window time_arrival <= now < time_arrival + time_holding.

When AttackMission destroys an ACS Defend fleet it sets processed=1 immediately (before time_arrival). Once time_arrival passed, the fallback condition time_arrival <= now became true and the destroyed mission was re-included as "active". checkEvents (called when the hold-end countdown expired) then found the mission as active and refused to remove the DOM rows, leaving them stuck.

Fix

Removed the stale orWhere fallback from getActiveFleetMissionsForCurrentPlayer(). The query now simply filters processed = 0.

This is safe because with the current architecture processArrival() fires at time_arrival (hold end) while processed=0 throughout hold — so the outbound is visible normally during travel and hold. The only time processed=1 is set before time_arrival is battle destruction, where the mission must disappear immediately.

Type of Change:

• Bug fix

Related Issues

Fixes #1321

Checklist:

• My code follows the style guidelines of this project
• - [x] I have performed a self-review of my code
• - [x] I have added tests that prove my fix is effective
• - [x] New and existing unit tests pass locally with my changes (php artisan test — 1041 passed)

[tiweb442]

Deployed on tiweb442 production pending merge

Applied the same one-line query simplification (processed = 0 only, stale ACS Defend orWhere removed) on space.tiweb-host.com for live testing.

Related tiweb442 PRs in the same batch: #1491, #1492, #1494. Email: #1375 (production-tested — comment).

Looks good to merge when maintainers have bandwidth — CI was green on last run.

[Geda173]

Hi @piciolo,

Sorry for the long wait time. I am slowly clearing the backlog of submitted PRs.
I have looked at the work and the fix is correct, but it needs the test the checklist says is here.

Two things before this can merge:

1. There's no test, though the checklist marks "I have added tests" as done. The diff is just the one-line query change. I verified the gap by restoring the old buggy query and running the ACS Defend, Attack, and MultiDefenderBattle suites. All 61 tests still pass. So nothing currently guards this behavior, and since this touches a query shared by every mission type in the fleet widget, a future change could silently bring the bug back.

Could you add a regression test along these lines?

• Dispatch an ACS Defend fleet with a hold time
• Destroy it in battle during hold
• Advance time past time_arrival (hold end)
• Assert getActiveFleetMissionsForCurrentPlayer() does not return the destroyed mission (and ideally assert a normal, non-destroyed ACS Defend fleet is still returned during hold, to lock in both sides)

2. This branch is ~4 months old, mind rebasing on main so CI runs against current checks (note phpstan is now level 8) before we merge.

[github-actions]

Preview deployment

Preview environment for this PR is ready for functional testing. Use one of the test accounts below to log in.

Status	URL	Commit	Last deployed
✅ Live	https://pr-1383.preview.ogamex.dev	86402ca	2026-07-25 13:43:43 UTC

Test accounts (click to expand)

Email (login)	Password	Role	Class	Planets	Description
test1@ogamex.dev	test	admin	Collector	2	Admin, high resources, mining focus
test2@ogamex.dev	test	player	General	1	Military focus, ships & fleet
test3@ogamex.dev	test	player	Discoverer	1	Exploration, probes & cargo ships
test4@ogamex.dev	test	player	None	1	New player, no class selected yet
test5@ogamex.dev	test	moderator	Collector	1	Moderator, balanced setup
test6@ogamex.dev	test	player	General	1	Vacation mode enabled
test7@ogamex.dev	test	player	General	3	Heavy defense across planets
test8@ogamex.dev	test	player	Discoverer	1	Fleet focus, inactive (i) 8 days
test9@ogamex.dev	test	player	Collector	1	Beginner, long inactive (I) 40 days
test10@ogamex.dev	test	player	General	5	Endgame, 90 unread messages

---

This preview will be automatically destroyed when the PR is closed.

[piciolo]

**Geda173 **

Hi thanks for the review — both points are addressed.

1. Regression test

Added two tests in tests/Feature/FleetDispatch/FleetDispatchMultiDefenderBattleTest.php:

• testDestroyedAcsDefendMissionNotReturnedByActiveFleetMissionsAfterHoldExpires — dispatches an ACS Defend fleet with a hold time, destroys it in battle during the hold, advances time past time_arrival (hold end) and asserts that getActiveFleetMissionsForCurrentPlayer() does not return the destroyed mission.
• testHoldingAcsDefendMissionReturnedByActiveFleetMissionsDuringHold — asserts that a normal, non-destroyed ACS Defend fleet is still returned while it is holding, to lock in both sides.

Why it guards the fix: a fleet destroyed in battle is marked processed = 1 (AttackMission::processArrival) while its time_arrival/time_holding are left unchanged. Under the current time model (time_arrival already includes the hold), the old orWhere clause matched such a mission in the [time_arrival, time_arrival + time_holding) window, so it re-surfaced in the widget for a full holding time after the hold ended. I confirmed the first test fails against the old query and passes with the processed = 0 fix. A normal holding outbound stays processed = 0, so it is still returned — that's what the second test locks in.

2. Rebase

Rebased onto the current main. CI is green, including phpstan level 8, the docker test suite, Pint and Rector.

[Geda173]

Thanks, looks good!