fix(cat): name every forget fact the call never attempted - #838
Merged
Conversation
forget_memories narrowed its own input three ways and told the user about none of them. forgetMemoriesMatching trimmed, dropped fragments under 4 characters and capped at 10; removeFromEconomicProfile did the same MINUS the cap; and the handler built its "no stored match" list by filtering the caller's raw strings against the trimmed facts the stores reported back. Each produced the same failure: "Removed X." while something the user named was never looked at. That is #563 finding 8's lie in a different costume — the user believes data is gone that is still sitting there. The padded case was the sharpest: " photography " could never appear in the unmatched list however plainly it missed, because raw was being matched against trimmed. One selector, selectForgetFacts → { wanted, tooShort, overCap }, used by both stores and the handler, so all three agree by construction and the leftovers have names. The reply now says what it did not attempt, on the nothing-was-removed path too, where the silence was most misleading. Counting the cap in usable facts also fixes a smaller bug: ten short fragments used to consume the whole budget and starve a real fact behind them. Mutation-proved: reverting stillUnknown to raw facts fails exactly the padded-fact test; removing the over-cap clause fails exactly the over-cap one. Refs #563 findings 10 and 11. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018waGt1ieA9TjpscqrbrnGb
catomean
added a commit
that referenced
this pull request
Aug 29, 2026
…ential (#847) Closes the tail of the #563 audit. FINDING 3 — an acknowledge flips an intent to buyer_confirmed and fires a "someone says they paid you" card into the seller's queue. One card is a prompt to check a wallet; a hundred is a denial-of-attention attack whose payoff is ship-the-goods-for-no-money. The budget in front of it was keyed on the caller, and the abuse shape is many addresses aimed at ONE seller. rateLimitPaymentClaim bounds it at 10/hour per recipient, asked once on the real transition — the idempotent re-claim paths return before it, cost an attacker nothing, and so must not spend a genuine payer's allowance either. The guard is INJECTED, not imported. Importing the limiter into the payments domain dragged Upstash's ESM build into eight domain test suites that have no business knowing about Redis — the layering violation announced itself. The HTTP layer owns the policy; the domain owns when it is asked. SUGGESTION 15 — the 402 body embeds `token` and WWW-Authenticate repeats it. That token is what a payer later exchanges for a receipt, so anything caching the response cached a credential for someone else's payment. Every other payment response set no-store; this one did not. SUGGESTION 14 — refreshPaymentStatus, reconcilePaymentIntent and resolveSellerWallet each took a supabase client they had stopped using; all three call getAdminClient() internally, so the signature advertised caller-scoped RLS behaviour that no longer existed. Removing it from resolveSellerWallet made resolveSellerReceiveInfo's client dead the same way, so that goes too. buyerConfirmPayment genuinely uses its client and is untouched — which is why the audit did not list it. SUGGESTION 17 — the semantic fallback and its 0.45 floor had no coverage at all: every forget test disables embeddings, so a silent revert to the old 0.75 would have changed which memories "forget that" reaches with nothing failing. Added a stem-sharing-innocents corpus too, since INCIDENT_CORPUS holds only targets and obvious strangers — a shape that flatters the matcher. One of those tests documents rather than changes behaviour: a single generic word is treated as a TOPIC, so "skills" clears every memory phrased with it. That is the deliberate containment rule (memory-forget.test.ts:138), not an over-match, and #838 makes it visible by naming each deleted memory. Refs #563 finding 3, suggestions 14, 15, 17. Claude-Session: https://claude.ai/code/session_018waGt1ieA9TjpscqrbrnGb Co-authored-by: Georgy Butaev <41178744+g-but@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes two findings from the #563 audit that share one root cause: three places
decided independently which of the user's facts a forget call would act on, and
they disagreed.
The bug
forget_memoriesnarrowed its own input three ways and told the user aboutnone of them:
forgetMemoriesMatchingtrimmed, dropped facts under 4 characters, andcapped at 10.
removeFromEconomicProfiletrimmed, dropped under 4 characters, and hadno cap at all — so a 12-fact request cleared 12 profile entries but only
10 memories, silently.
raw strings against the trimmed facts the stores reported back. A
padded
" photography "could therefore never appear in that list, howeverplainly it had missed.
Every one of these produces the same failure:
🧹 Removed X.while somethingthe user named was never looked at. That is finding 8's lie in a different
costume — the user believes data is gone that is still sitting there.
The fix
One exported selector,
selectForgetFacts(facts) → { wanted, tooShort, overCap },used by both stores and the handler, so all three agree by construction and the
leftovers have names instead of vanishing. The reply now says what it did not
attempt:
most misleading.
Counting the cap in usable facts also fixes a smaller bug: ten short
fragments used to consume the whole budget and starve a real fact behind them.
Verification
9 new tests in
forget-reports-dropped-facts.test.ts. Mutation-proved — revertingstillUnknownto filter raw facts fails exactly the padded-fact test; removing theover-cap clause fails exactly the over-cap test. Full Cat suite green (52 suites,
727 tests) and a full non-incremental
type-checkclean. The box was tooloaded to finish one
npm run verify, so it was verified by parts instead:eslint on the changed files, and check:sizes / audit:routes / duplication /
dead-fields / client-ip / user-scoped-deletes / one-current-user all green.
The pre-push hook (full type-check + tests) also passed. CI runs the whole
bundle.
Refs #563 findings 10 and 11.