[codex] fix autopilot poison summit triggers

[loothero]

Summary

• Make autopilot poison checks re-run when Summit beast state, poison balance, caps, thresholds, or action blockers change.
• Guard targeted poison so wider effect dependencies do not repeatedly apply poison to the same target.
• Stop conservative poison from falling through into attack logic in the same evaluation pass.
• Clear the potion-applying state when poison or extra-life actions fail.
• Add hook tests for missed poison trigger scenarios.

Root Cause

The autopilot poison effects returned early while applyingPotions or attackInProgress were true, but those blocker states and other poison eligibility inputs were not effect dependencies. If the Summit beast changed while autopilot was busy, or if poison balance arrived after the Summit update, the poison logic could stay idle while the UI showed “Waiting for trigger...”.

Validation

• pnpm exec vitest run src/hooks/useAutopilotOrchestrator.test.tsx
• pnpm exec vitest run src/hooks/useAutopilotOrchestrator.test.tsx src/contexts/GameDirector.test.tsx
• pnpm exec tsc --noEmit
• pnpm exec eslint . --quiet
• pnpm build

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
summit	Ready	Preview, Comment	May 4, 2026 1:54am

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a9794dc1-29b7-461c-9ff4-87eadf00b9f8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/autopilot-poison-summit-change

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[railway-app]

🚅 Deployed to the summit-pr-149 environment in Summit

3 services not affected by this PR

• summit-db
• summit-indexer
• summit-api

[gemini-code-assist]

Code Review

This pull request enhances the autopilot poison logic within useAutopilotOrchestrator by introducing targetedPoisonKeyRef to prevent duplicate poison applications and updating handleApplyPoison to handle asynchronous results and state resets. The GameDirector context was updated to ensure applyingPotions is reset on failed actions, and a comprehensive test suite for poison triggers was added. Feedback suggests refining the targetId check to handle both null and undefined values to prevent potential issues with executeGameAction.

[gemini-code-assist]

The check targetId === undefined is slightly too restrictive. If summit?.beast?.token_id is null (which can occur depending on the API response or state initialization), targetId will be null. Since null === undefined is false, the function would proceed, potentially passing null to executeGameAction. Using targetId == null is safer as it correctly handles both null and undefined while still allowing 0 if it's a valid ID.

Suggested change

	if (targetId === undefined || applyingPotions || amount === 0) return false;
	if (targetId == null || applyingPotions || amount === 0) return false;

[codecov]

Codecov Report

❌ Patch coverage is 68.13187% with 29 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
client/src/hooks/useAutopilotOrchestrator.ts	70.45%	26 Missing ⚠️
client/src/contexts/GameDirector.tsx	0.00%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

Claude Review - React/Frontend Review

run=25297281969 attempt=1 sha=510eeda96f37b3e32909eeae8e9c99eca2106021 scope=client

Findings

[MEDIUM] client/src/hooks/useAutopilotOrchestrator.ts:314 - Inconsistent owner address normalization in targetedPoisonKeyRef key
Impact: The player-targeted poison key uses summit.owner.toLowerCase() while elsewhere in the file (line 261) the code normalizes with .replace(/^0x0+/, '0x').toLowerCase(). If the same logical address arrives from different sources with different leading-zero padding (Cartridge, indexer, on-chain), the key will not match across renders for the same target — defeating the dedup intent and re-applying poison after every render that flips a dependency, until applyingPotions/in-flight tx state intercepts.
Fix: Normalize with the same helper used by summitOwnerIgnored, e.g. const ownerKey = summit.owner.replace(/^0x0+/, '0x').toLowerCase(); and embed ownerKey into the poisonKey.

[MEDIUM] client/src/hooks/useAutopilotOrchestrator.ts:107-134 - setApplyingPotions(false) is split between GameDirector (success-path resolved-false) and the orchestrator's .catch (rejection); the .then falsy branch does not reset it itself
Impact: Correct behavior depends on GameDirector clearing applyingPotions on every failure path of add_extra_life/apply_poison. If a future GameDirector edit returns a falsy result without going through the new line 668-670 branch (e.g., a guard that return falses before reaching executeAction), the orchestrator will be permanently stuck in applyingPotions=true, and "Waiting for trigger..." will recur — exactly the failure mode this PR is fixing. There is also no symmetric reset in the synchronous early-returns inside GameDirector's poison/extra-life branches (e.g. applyPoison's missing beastId at line 630-633).
Fix: Make the orchestrator's .then(result => { if (!result) { ...; setApplyingPotions(false); } }) self-sufficient so it does not rely on GameDirector's internal reset, or audit every early-return inside GameDirector that returns false for these action types and ensure each clears applyingPotions.

[LOW] client/src/hooks/useAutopilotOrchestrator.ts:118-131 - Stale-closure ref clearing in .then/.catch can erase a newer targeted-poison key
Impact: Sequence: targeted poison fires for beast A → targetedPoisonKeyRef = "beast:A:..." → tx fails → setApplyingPotions(false) from GameDirector → effect re-runs → summit changes to beast B → ref cleared by line 243 effect → new targeted poison fires for B → targetedPoisonKeyRef = "beast:B:...". If the original action 1 promise resolves later (microtask ordering with setState batching can put it after the new render), its .then unconditionally writes targetedPoisonKeyRef.current = null, allowing a duplicate poison call against beast B on the next dependency change. Low likelihood given applyingPotions gating in handleApplyPoison, but the ref clear is intentionally racy.
Fix: In .then/.catch, only clear targetedPoisonKeyRef when it still matches a key tied to this in-flight target, e.g. capture const keyAtDispatch = targetedPoisonKeyRef.current after dispatch and reset only when targetedPoisonKeyRef.current === keyAtDispatch.

[LOW] client/src/hooks/useAutopilotOrchestrator.ts:339-358, 423-452 - Effects depend on the entire collection array reference
Impact: Both poison effects now list collection in deps. Any unrelated mutation in useGameStore that creates a new collection array (rewards updates, beast-stat refreshes from WS, even an unchanged-content reassignment) will re-run the effect. Combined with the very wide dep arrays (≈18 entries on the main effect), this materially increases evaluation frequency and the chance of overlapping in-flight checks. Functional behavior is protected by the refs and applyingPotions, so this is mainly a perf/log-noise concern.
Fix: Derive a narrow primitive — e.g. const isOwnSummit = useMemo(() => collection.some(b => b.token_id === summit?.beast?.token_id), [collection, summit?.beast?.token_id]) — and depend on isOwnSummit instead of collection.

[LOW] client/src/hooks/useAutopilotOrchestrator.ts:301, 314 - poisonKey does not include poisonPotionsUsed/poisonBalance
Impact: After a successful targeted poison, poisonPotionsUsed increments and poisonBalance drops, but the dedup key is unchanged, so subsequent renders cannot re-trigger — which is desired. However, if a partial application happened (e.g. clamped by remainingCap or poisonBalance) and later the cap/balance increases, the key still matches and the topup will not fire even though the user's playerAmount policy is unmet. If "apply full targeted amount when capacity becomes available" is the intent, the key should reflect the spent-vs-policy gap.
Fix: If progressive top-up is desired, factor min(playerAmount, poisonBalance, remainingCap) (the actually-applied amount, or remaining gap) into the key; if not desired, no change needed but document the one-shot semantic.

[INFO] client/src/hooks/useAutopilotOrchestrator.ts:109 - targetId === undefined guard now allows targetId === 0
Impact: Improvement over the previous !targetId, which incorrectly rejected token_id === 0. No issue — flagging because this is a deliberate behavior change worth verifying against the contract's token-id space (token id 0 does seem possible in the indexer, see the test using token_id: 1 upward).
Fix: None required; keep as-is.

[INFO] client/src/hooks/useAutopilotOrchestrator.test.tsx:1-219 - Tests cover the two main regression scenarios but not the new conservative-poison early return or the GameDirector failure-clears-applyingPotions branch
Impact: Other behavioral changes claimed in the PR description (conservative poison no longer falling through into attack logic; applyingPotions cleared on add_extra_life/apply_poison failure) lack direct tests. Future regressions in those branches won't be caught.
Fix: Add a third test that asserts attack_until_capture/attack is NOT dispatched in the same render after a conservative poison fires, and a small GameDirector test asserting setApplyingPotions(false) is invoked when executeAction returns nothing for a poison action.

Residual Risks / Testing Gaps

• No browser smoke check verifying the "Waiting for trigger..." UX no longer sticks across the targeted scenarios.
• The wider dep arrays should be observed under real WS update churn — concern is repeated effect runs causing log thrash, not correctness.
• Address-normalization inconsistency (MEDIUM above) is not exercised by the new tests since the test seeds owner: "0xabc" directly.

Summary

Summary: 0 CRITICAL, 0 HIGH, 2 MEDIUM, 3 LOW, 2 INFO

[github-actions]

Codex Review - React/Frontend Review

[INFO] client/:1 - Review could not be completed from the available workspace.
Impact: The required git diff 90301c58564ebac000d30f2692d409db8eec6cde...510eeda96f37b3e32909eeae8e9c99eca2106021 -- client/ command failed before execution with bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted, so I could not inspect the PR-only diff or independently verify the PR assumptions/workarounds.
Fix: Re-run the review in an environment where local shell access can execute, or provide the client/** diff directly.

Summary: 0 CRITICAL, 0 HIGH, 0 MEDIUM, 0 LOW, 1 INFO