Skip to content

fix(drops): stop false campaign completion from sibling-campaign session drops - #19

Open
1p0d wants to merge 1 commit into
miwidot:mainfrom
1p0d:fix/false-campaign-completion
Open

1p0d wants to merge 1 commit into
miwidot:mainfrom
1p0d:fix/false-campaign-completion

Conversation

@1p0d

@1p0d 1p0d commented Sep 11, 2026

Copy link
Copy Markdown

Fixes #18. Refs #14.

Bug

While a campaign is being farmed, DropCurrentSessionContext can report a drop that belongs to a sibling campaign of the same game. PollProgressOnce compared the picked campaign against that drop's requirement (648/30 instead of 648/660) and ran the "finished externally" check every minute. That check only needs one inventory response without data to mark the campaign completed for good, and GetDropsInventory silently read such a response as "nothing in progress". Full evidence is in #18.

Fix

1. Only use the session for the campaign that owns its drop (internal/drops/progress.go)

  • campaignOwnsDrop(campaignID, dropID) checks the campaign cache.
  • progressFromSession builds the progress update. When the drop belongs to the picked campaign, behaviour is unchanged. When it doesn't, only the minutes are applied (no drop ID, no required minutes) and the completion check is skipped.
  • The minutes are still applied on purpose. The session's minute count kept tracking the picked campaign: three of its drops were claimed normally while the bug was active. Dropping the update would trip the silent-pick detector and switch channels every 3 minutes.
  • The file log records [Drops/Poll] session drop <id> is not part of campaign <id> — minutes applied, completion check skipped.

2. A failed inventory fetch is an error (internal/twitch/drops.go)

  • Parsing moved into parseInventoryResponse. A missing or null currentUser or inventory now returns an error instead of nil, nil, nil. An inventory with nothing in progress still parses as before.
  • GetDropsInventory returns the inventory error instead of discarding it. Both callers already handle errors: ProcessDrops logs it and skips the cycle, and MarkCompletedIfFinishedExternally returns without marking.
  • Trade-off: when the inventory fetch fails, the whole cycle is skipped, dashboard data included. The worst case is one delayed cycle instead of a campaign completed forever.

Not in this PR: a way to recover campaigns that are already falsely completed. That's a design decision, so it's a recommendation in #18.

Tests

New in internal/drops/progress_session_test.go:

  • TestProgressFromSession_SiblingCampaignDropNeverCompletes: the exact case from the issue (a sibling drop at 648/30) never signals completion and doesn't pass on the sibling's drop ID or required minutes.
  • TestProgressFromSession_OwnedDrop: 647/660 isn't done, 660/660 and 700/660 are, and a required value of 0 never is.
  • TestCampaignOwnsDrop: the campaign's own drop, a sibling's drop, and a campaign missing from the cache.
  • TestApplyProgressUpdate_SiblingSessionKeepsRowRequired: the dashboard row keeps Required=660 instead of switching to /30.

New in internal/twitch/inventory_parse_test.go:

  • TestParseInventoryResponse_MissingDataIsAnError: a missing or null currentUser or inventory.
  • TestParseInventoryResponse_NothingInProgress: a valid but empty inventory still parses.

go vet ./... is clean, and go test ./... passes before and after the change.

Verified live

Running on linux/arm64 (Raspberry Pi 4, Docker) after removing the affected campaign from completed_campaigns:

  • Twitch kept returning the sibling drop for the whole run: the completion check was skipped 238 times over 4 hours, with 0 false completions.
  • Early Access 0.1 resumed and ran to the end: Silver Drop 2 (660 min), Gold Drop 4 (780 min) and Gold Drop 5 (900 min) were claimed normally.
  • After the last claim, the regular claim pass marked the campaign completed (fully claimed — marked as completed) once Twitch had removed it from the in-progress list. Twitch shows all 9 rewards owned, and the farmer moved on to the next queued campaign.
  • No inventory fetch failed during the run, so the error path in fix 2 is covered by the unit tests only.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KgmYh9TGJcPzjbAd2sCTqm

DropCurrentSessionContext can report a drop that belongs to another
campaign of the same game. PollProgressOnce treated it as the picked
campaign's drop, compared the minutes against its requirement (648/30
instead of 648/660) and ran MarkCompletedIfFinishedExternally every
minute. A single inventory response without data then marked the
campaign completed, because GetDropsInventory discarded the inventory
error and every campaign read as not in progress.

- Only run the completion check when the session's drop belongs to the
  picked campaign; otherwise apply the minutes alone.
- Treat a missing currentUser or inventory in the response as an error
  and return it from GetDropsInventory.

Fixes miwidot#18
Refs miwidot#14

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KgmYh9TGJcPzjbAd2sCTqm
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 2628ab1d-bd51-4675-8ceb-5ecb7300d66a


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

Campaign falsely marked COMPLETED mid-progress (root cause of #14, still reproduces on v2.3.0)

1 participant