Skip to content

fix: prevent alert update from wiping subjects#296

Merged
kome12 merged 1 commit intomainfrom
fix/alert-update-wipes-subjects
Mar 16, 2026
Merged

fix: prevent alert update from wiping subjects#296
kome12 merged 1 commit intomainfrom
fix/alert-update-wipes-subjects

Conversation

@0xlaveen
Copy link
Contributor

Summary

  • Bug: Updating a common-token-transfer alert with any flag (e.g. --usd-min 5000) wiped existing subjects, causing API rejection: "Common Token Transfer Alert must specify subject or tokens"
  • Root cause: buildCommonTokenTransferData unconditionally set data.subjects = subjects ?? [] (line 170), so when --subject wasn't passed, subjects defaulted to []. On the update path (applyDefaults: false), this empty array overwrote existing subjects via shallow merge at line 568.
  • Fix: Changed to if (subjects) data.subjects = subjects; — matching the pattern already used by counterparties on line 173. The create path still works because TYPE_DEFAULTS supplies subjects: [] via applyDefaults.

Test plan

  • Added test: should not wipe subjects when updating common-token-transfer without --subject
  • All 344 existing tests pass

🤖 Generated with Claude Code

buildCommonTokenTransferData unconditionally set data.subjects = subjects ?? [],
causing the update path (applyDefaults: false) to overwrite existing subjects
with an empty array via shallow merge. Now subjects is only set when the user
explicitly passes --subject, matching the pattern used by counterparties.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@nansen-pr-reviewer
Copy link

pr-reviewer Summary

No issues found

The code review completed successfully with no findings.

Review effort: 1/5 (Trivial)

Summary

This is a clean, minimal bug fix that prevents alert updates from inadvertently wiping a user's existing subjects configuration when --subject is not passed on the CLI.

Root cause: buildCommonTokenTransferData previously used data.subjects = subjects ?? [], which would always set an empty subjects array when no --subject flag was provided. During an update (applyDefaults: false), this empty array would be merged into the existing alert payload (via { ...existing.data, ...builtData }), overwriting whatever subjects the user had previously configured.

Fix: The one-line change to if (subjects) data.subjects = subjects correctly mirrors how counterparties, chains, tokens, and all other optional fields are handled throughout the same function — only include the field in the patch payload when the user explicitly provided it.

The accompanying test accurately simulates the update flow (calling buildAlertData with applyDefaults: false) and confirms subjects is undefined in the result, so it won't clobber server-side state. No issues found.


Token usage: 68 input, 2,739 output, 125,028 cache read, 16,147 cache write

@kome12 kome12 merged commit cdf18eb into main Mar 16, 2026
7 checks passed
@kome12 kome12 deleted the fix/alert-update-wipes-subjects branch March 16, 2026 01:04
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