Skip to content

feat(action-token): reserve link tokens with transfer_pending - #579

Merged
mahdi2ba merged 3 commits into
Greenstand:keycloakfrom
samwel141:feat-link-reservations-574
Sep 21, 2026
Merged

mahdi2ba merged 3 commits into
Greenstand:keycloakfrom
samwel141:feat-link-reservations-574

Conversation

@samwel141

Copy link
Copy Markdown
Collaborator

Depends on #577. The diff shows both commits until that merges.

Problem

Two reservation mechanisms existed and neither saw the other. A share link recorded its tokens in action_token, which only link generation consulted, while every transfer path honours token.transfer_pending. So a normal send took the token a live link had promised, and the link died at claim with a 409 the sender never saw.

Fix

Reuse the flag every path already honours, rather than teaching each path about action_token.

  • New nullable action_token_id column on token. Additive, so v1 and v2 ignore it. transfer_pending_id is left alone: it is read elsewhere as a transfer id, and keeping it null for link tokens means a transfer cancel can never release a link's tokens by accident.
  • generate() claims the tokens with a conditional update and requires the full count, so two concurrent creations cannot both take the same row.
  • cancel() and redeem() hand them back, redeem inside its own transaction so a link whose record cannot be read fails closed.
  • Expired links release lazily on any generate() or list() by anyone, so no cron job is needed.

One fix that was not in the issue

redeemActionToken created new TokenService(), which opens its own Session outside the open transaction, so it could not see the release. It now reads on the transaction's session.

Regression notes

A pending normal send reserves and releases exactly as #560 does, transfer_pending_id untouched. fulfillTransfer picks only free tokens, so it can no longer take a link's token. Links created before this deploy have a row but unflagged tokens, and keep redeeming; they are simply unprotected until claimed or cancelled.

actiontoken-skip-reserved.spec.js from #577 now asserts the new behaviour: promised tokens are flagged and carry the link's id.

Testing

Six new integration specs: link and send take different tokens and the link still redeems; the last free token being promised refuses a send; cancel frees it; two concurrent links leave exactly one alive; an expired link releases on the next link activity; redeem clears the flag.

Unit 241 passing, integration 141 passing across three runs.

Refs #574

Share link generation took the first N tokens of the wallet, reserved or
not, while every transfer path skips reserved ones. The link was issued
with no warning and redeem refused it with a 409 that landed on the
recipient, who could do nothing about it.

Add Token.getAvailableTokens, filtering transfer_pending and claim, and
page over that when picking a bundle. The existing count check then
produces the 409 at generate time, where the sender can act on it.
getByOwner is untouched, so GET /tokens still shows a wallet's full
holdings.

Apply the same rule to the explicit ids path, which checked ownership
only. This is what Transfer.transferActionToken already enforces at claim
time, moved earlier.

Refs Greenstand#571
Two reservation mechanisms existed and neither saw the other. A share
link recorded its tokens in action_token, which only link generation
consulted, while every transfer path honours token.transfer_pending. So a
normal send took the token a live link had promised, and the link died at
claim with a 409 the sender never saw.

Reuse the flag every path already honours rather than teaching each path
about action_token:

- New nullable action_token_id column on token, additive so v1 and v2
  ignore it. transfer_pending_id is left alone: it is read elsewhere as a
  transfer id, and keeping it null for link tokens means a transfer
  cancel can never release a link's tokens by accident.
- generate() claims the tokens with a conditional update and requires the
  full count, so two concurrent creations cannot both take the same row.
- cancel() and redeem() hand them back, redeem inside its own transaction
  so a link whose record cannot be read fails closed.
- Expired links release lazily on any generate() or list() by anyone, so
  no cron job is needed.

redeemActionToken now reads its tokens on the transaction's own session.
A separate TokenService opens its own connection and would not see the
release.

Refs Greenstand#574
generate() flags the tokens first and inserts the action_token row after.
If the conditional update flagged fewer rows than asked (a concurrent link
or send took some between selection and update), or the insert failed, the
flagged rows kept an action_token_id that no row will ever carry. Cancel,
redeem and the lazy expiry all release by that id through the row, so those
tokens stayed unsendable for good.

Release by the id on both failure paths before rethrowing.

Refs Greenstand#574

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@mahdi2ba
mahdi2ba merged commit 96c0ba3 into Greenstand:keycloak Sep 21, 2026
1 check passed
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.44.0-keycloak.30 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

mahdi2ba added a commit that referenced this pull request Sep 21, 2026
…elf-healing

Follow-ups to #579 (#574), from its review.

- generate() flags the tokens and inserts the link row in one transaction.
  A short reservation (a concurrent link or send got there first) or a
  failed insert rolls the flags back, so no token can stay flagged for a
  link that does not exist. The UPDATE is also scoped to the sender wallet.
- Explicit token ids must belong to the sender wallet. walletB could name a
  token of walletC, a wallet it manages, without sender_wallet; the link
  then spoke for walletB and claim time refused it forever. Refuse at
  generate time, to the sender, with a clear message.
- Housekeeping releases every token flagged for a link that is not active,
  in one statement (NOT EXISTS active row), instead of a per-id loop after
  expiry. That also heals whatever a crash between two statements left
  behind. cancel() updates state and releases in one transaction.
- New migration turns token_action_token_id_idx into a partial index on
  action_token_id IS NOT NULL: nearly every row is NULL, so the index stays
  tiny and off the write path of ordinary token updates.
- GET /action-tokens?state=expired is accepted; rows can be expired since
  #579.

Three integration specs: an explicit token of a managed wallet without
sender_wallet is refused and flags nothing; a token flagged for a link with
no row is released on the next link activity; expired links list by state.

Refs #574

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@dadiorchen dadiorchen self-assigned this Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants