Skip to content

fix(cart): verify cart remove only removes the target line #41 - #49

Merged
yashiels merged 1 commit into
mainfrom
agent/cart-guard
Sep 26, 2026
Merged

yashiels merged 1 commit into
mainfrom
agent/cart-guard

Conversation

@yashiels

Copy link
Copy Markdown
Owner

cart remove / set-qty read the cart before and after the write. A sku not in the cart is refused (exit 4, no DELETE). If any other line disappears or changes quantity (per-SKU summed quantities, so duplicate lines are covered) the command prints unexpectedRemovals plus restore commands and exits 1; it never auto-writes. Dry run lists the lines that would remain; --verbose logs customerId, auth generation and the DELETE body.

The DELETE body already matches the 4.3.0 app, so no body defect was found for the original report.

Live on the real account: added sku 83379710 to a 3-line cart and removed it, cart back to exactly the original 3 lines; absent sku exits 4. 357 tests. Oracle signed off (2 rounds).

Closes #41

@yashiels
yashiels merged commit 16d83ab into main Sep 26, 2026
1 check passed
@yashiels
yashiels deleted the agent/cart-guard branch September 26, 2026 23:36

@sentinel-skyner sentinel-skyner Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Sentinel review

Found 2 correctness issues in cart guard behavior.

2 inline comments below.

Automated review via cursorbridge:gpt-5.3-codex. Findings only — not an approval. Push new commits to re-review.

Comment thread src/commands/cart.ts
ctx.logger.info(`➖ Removing SKU ${skuId}…`);
await ctx.client.removeCartItem(skuId);
const cart = await ctx.client.getCart();
const unexpectedRemovals = changedLines(before, cart, skuId, null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ High Passing null as targetQuantity disables verification that the target SKU was actually removed. If removeCartItem is a no-op and the SKU remains, unexpectedRemovals stays empty and the function reports success.

Suggested change
const unexpectedRemovals = changedLines(before, cart, skuId, null);
const unexpectedRemovals = changedLines(before, cart, skuId, 0);

Comment thread src/commands/cart.ts
: `takealot cart add --sku ${line.skuId} --qty ${line.quantity} --confirm --yes`;
ctx.logger.warn(`Restore ${line.title}: ${command}`);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Medium Restore commands use line.quantity, but changedLines deduplicates by SKU and keeps only one representative line. For duplicated SKUs, this quantity is not the original per-SKU total, so the suggested set-qty command can restore the wrong amount.

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.

cart remove once left the whole cart empty (remaining 0); add a guard against collateral removals

1 participant