fix(security): throttle the public exportTraces endpoint - #4228
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b4977079f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
exportTraces was the only export operation that enqueued an email export without passing through enforceExportRequestRateLimit, so an API caller could enqueue unbounded CSV export jobs and emails per (org, project, recipient) while exportSignals and exportDatasetRows were both capped at 10/hour in production. Adds the same throttle its siblings use and documents the resulting 429 in the OpenAPI contract, which required swapping typedResponses (success status only, by design) for the explicit responses object both siblings already use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
andresgutgon
force-pushed
the
cursor/critical-bug-management-9b04
branch
from
August 3, 2026 07:57
4b49770 to
1e41c20
Compare
andresgutgon
changed the title
Fern-generated from the openapi.json change in the previous commit, plus the version bumps that gate publishing: the SDKs publish off their manifest version, the CLI off the top entry in its CHANGELOG. Mirrors #4212, which did the same for the datasets and signals exports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
andresgutgon
changed the title
csansoon
added a commit
that referenced
this pull request
Aug 3, 2026
The rebase left `openapi0.json` conflicted, and taking either side by hand was wrong: it dropped the `429` export-rate-limit response that #4228 added on development. Regenerating from the emitted spec restores it alongside the entity-enum change from this branch.
csansoon
added a commit
that referenced
this pull request
Aug 3, 2026
* test(redaction): pin detector accuracy with a labelled corpus
Measures the deterministic PII detectors against 117 labelled occurrences and
30 clean texts, and asserts the outcome the engine produces today rather than
the one we want. 41 occurrences are stored verbatim, 5 survive partially, 4
land under the wrong entity, and 12 false positives are accepted.
Asserted case by case with pinned totals rather than against a recall
threshold, matching the one regression suite the repo already has
(packages/domain/flaggers/src/use-cases/regression). Closing a gap therefore
fails this suite until the row and the total are both edited, which keeps an
improvement as visible in the diff as a regression.
Three derived suites come with it:
- a punctuation sweep over 13 identifier shapes and 16 wrappers, which is how
the trailing-period class of bug becomes a list instead of an anecdote;
- seeded collision rates on synthetic identifiers, recording that base58 and
40-char hex shapes are redacted unconditionally because no checksum is
validated;
- a backtracking canary bounded far above the measured cost, since span
content is attacker controlled and the ingest path is synchronous.
resolveOverlaps is exported so the corpus observes the same accepted-match set
the redaction pass does, rather than reimplementing the resolution and drifting
from it.
* fix(redaction): stop a trailing period from disabling card, phone and IP detection
"My card number is 4111111111111111." was stored verbatim. So was a NANP phone
number or an IPv4 address at the end of a sentence, and every one of the three
followed by an ellipsis.
The trailing guards rejected any following dot outright. They exist to keep the
detectors out of decimals and version strings, but a lookahead cannot tell
`1111.` from `1111.5`, and backtracking does not rescue it: every shorter run
of digits is then followed by a digit, so the whole match fails. Rejecting a
dot only when a digit follows keeps the decimals out and lets the sentence
through.
The leading lookbehinds are untouched. They are what keeps the card detector
out of `3.14159265358979` and `0.000000000000001`, now asserted from the other
side too with `ratio 0.4111111111111111`.
Closes 4 corpus gaps and takes the punctuation sweep from 18 failing
combinations to 4, all of which are leading-dash forms handled in later
commits.
* fix(redaction): reject invalid NANP area and exchange codes
The separated NANP form was "three numbers of length 3, 3 and 4", which is also
the shape of a row count, a grid offset and a part number. It was redacting
`100 200 3000`, `123 456 7890` and `100-200-3000` out of ordinary tool output.
Area and exchange codes in the NANP both start at 2. Requiring that removes
three of the five measured phone false positives and costs no true positive,
since every real area and exchange code in the corpus already satisfies it.
`250 300 1000` (p50/p95/p99 latencies) and `234 567 8901` (a thousands-grouped
amount) still match and are still recorded as accepted false positives. Those
two are genuinely indistinguishable from a phone number without reading the
surrounding prose, so no pattern will fix them.
* feat(redaction): detect internationally formatted phone numbers
Phone was the weakest detector at 36% recall. E.164 only matched contiguous
digits, so every number written the way people write them was stored verbatim:
`+1-415-555-2671`, `+44 20 7183 8750`, `+34 600 123 456`, `+91 98765 43210`,
`+46 70 123 45 67`, `+81 90 1234 5678`. `+1 415 555 2671` was worse than
missed, since NANP matched its last ten digits and left the country code in the
span.
One pattern per separator, so a match cannot bridge two numbers formatted
differently, which is the same reason the card and IBAN detectors are split.
Three constants are load-bearing and each came from a failing case:
- `[1-9]` for the country code, because no calling code starts with zero and
`+0 123 4567` matched without it;
- four groups rather than three, because `+46 70 123 45 67` has five components
and a cap of three left the last two digits behind;
- a 20-digit ceiling rather than E.164's 15. The repetition is greedy, so
`+44 20 7183 8750 4471` runs into the following number and reaches 16 digits.
Rejecting there discards the match and stores the phone number, because a
validator cannot shorten a match the regex has committed to. Accepting it
over-redacts the adjacent number instead.
The NANP pattern also takes an optional leading `1` trunk code, so
`1-415-555-2671` is matched whole. That widens one known false positive:
`1 234 567 8901` in a thousands-grouped amount now absorbs the leading digit.
Closes 8 corpus gaps and one partial. National forms with no `+` (`07700
900123`, `06 12 34 56 78`) and the IDD prefix form remain out of scope.
* feat(redaction): add the missing vendor token prefixes
HuggingFace, GitLab, npm, Google OAuth and SendGrid tokens were stored
verbatim, as were Slack webhook URLs, whose path segments are themselves the
credential.
They share one alternation rather than getting a detector each, because every
detector is another pass over every string leaf and this runs on the ingest hot
path.
Also tightens `looksLikeLongToken`. `sk-learn-tutorial-notebook-v2-final` was
redacted as a secret: its tail is exactly 32 characters and contains a digit, so
both existing gates passed. A hyphenated all-lowercase tail is a slug rather
than a key, since real keys are base62 and mix case, or carry no hyphens at all.
Closes 5 corpus gaps and removes the last secret false positive.
* feat(redaction): rank detectors, and detect DSN credentials
A database password was being stored as `[REDACTED_EMAIL]`. In
`postgres://user:pass@db.internal` the password is also a valid email local
part, the email match covers more characters, and overlap resolution was
leftmost-longest, so the email detector won and labelled a credential as an
address. The same happened to a GitHub token in a `https://token@github.com`
remote.
Worse, the accident was load bearing. With an IP host —
`postgres://app:Sup3rS3cret@127.0.0.1:5432/app`, the shape in every
ECONNREFUSED stack trace — the email detector has nothing to match and the
password was stored verbatim.
So overlap resolution gains a rank, applied before extent: a detector that
identified what it matched beats one that happened to cover more of it. Only the
two detectors with a demonstrated collision carry it, so the ranking stays
evidence-driven rather than decorative.
The DSN detector matches the credential alone rather than the whole URL, which
keeps the host, port and database name readable in a stack trace. A well-formed
URL must percent-encode an `@` in its password, so the password may not contain
one.
Closes 1 corpus gap and 2 of the 4 mislabelled matches. The corpus GitHub token
was also lengthened to 36 characters after the prefix, which is what real ones
carry; the short value never reached the github detector at all.
* feat(redaction): detect credentials from their assignment key
Secret sat at 50% recall because the credentials that leak most have no shape to
match: `POSTGRES_PASSWORD=…`, `Authorization: Bearer …`, `{"api_key":"…"}`, an
AWS secret access key, a base64 value in a Kubernetes Secret. This is what §5's
ban on entropy heuristics leaves on the table, and the way to get it without
guessing is to read the key rather than the value. Nothing here looks at how
random a value is.
Detectors gain an optional capture group so a pattern can require context it
must not redact, which keeps the placeholder over the credential and leaves the
key readable. A lookbehind would express the same thing but is evaluated at
every position in the leaf; a pattern opening with literal alternatives lets the
engine skip. Measured on a 32 KB leaf that is the difference between 1.07 ms and
a cost we could not afford at concurrency 50.
Four guards, each from an observed false positive:
- Plural `tokens` is not a credential key. `max_tokens`, `prompt_tokens` and
`total_tokens` are in nearly every LLM span we ingest.
- A bare `key` is not a credential key, only a qualified one. `idempotency_key`,
`partition_key`, `cache_key` and `sort_key` are all over tool output.
- A dotted or bare identifier with no digit is a reference, not a credential:
`const apiKey = options.apiKey` and `api_key: process.env.OPENAI_API_KEY` were
both being redacted.
- The value may not start with `(`, or `# TOKEN= (unset)` redacts `(unset`.
The separator is `[ \t]*` rather than `\s*` so a match cannot cross a newline
and take the next line of a YAML block as its value. The leading-`[` rejection
also keeps redaction idempotent, since `[REDACTED_SECRET]` must not re-match.
Closes 5 corpus gaps. A credential under a key named only `*_KEY`, with no
`api`, `secret` or `private` in the name, is still missed; that is the price of
excluding bare `key` and it is recorded in the corpus.
* fix(redaction): tighten the email domain gate and widen the local part
Two precision fixes and two recall fixes in one detector.
`logo@2x.png` and `bundle@main.tar` were redacted as email addresses. They
satisfy every structural rule one has, and asset naming conventions put them in
real tool output. A two-label domain whose TLD is a file extension is rejected
now; the rule is scoped to two labels because `/home/user/mail@example.com.txt`
is a real address followed by an extension.
On the recall side, a local part the class could not express was worse than a
miss, because the match started partway in and stored the name it was meant to
remove: `María.Garcí[REDACTED_EMAIL]` and `O'[REDACTED_EMAIL]`. Non-ASCII letters
and the apostrophe are part of the local part now. Percent-encoded addresses are
matched too, which is how a reset link carries one.
The first character may not be an apostrophe, so a single-quoted address in code
or YAML is matched from the local part rather than from the quote. It may still
be `+`: excluding it made the email match shorter than the phone match inside
it, and `+14155552671@example.com` started leaving the domain beside a
`[REDACTED_PHONE]`.
Both halves are bounded to their RFC 5321 limits. `-` is in both classes, so the
unbounded form backtracked one character at a time from every offset over a long
run of dashes, quadratic in the size of the leaf. The backtracking canary added
with the corpus caught it at 307 ms; bounded, the same input scans in 1.9 ms.
Closes 1 corpus gap and both remaining email partials, and removes 3 of the 8
accepted false positives.
* feat(redaction): close the IBAN, card, SSN and IPv6 format gaps
Seven format gaps, each a value a customer would reasonably expect to be
covered by a detector that already exists.
IBAN matching is case-insensitive now, because customers paste lowercase, and
accepts dash grouping as printed on invoices. The mod-97 checksum is what makes
the wider net affordable: it rejects 96 of every 97 candidates, which matters
because lowercase alphanumeric runs are far more common in tool output than
uppercase ones.
Maestro and UnionPay prefix ranges were missing, so Luhn-valid cards from both
were stored verbatim. Card groupings accept slashes, which is how a card
handwritten onto a form gets transcribed.
SSNs accept dot separators. The 9xx area exclusion was dropped and replaced with
a check against the ITIN group ranges the IRS assigns: no SSN has a 9xx area,
but every ITIN does, and an ITIN identifies a taxpayer just as well. A 9xx area
outside those ranges is still rejected.
The SSN boundary guards now match the shape used elsewhere, which also closes
the last two entries in the punctuation sweep: `-123-45-6789` and
`123-45-6789-suffix` were both stored verbatim.
IPv6 addresses that compress from the left (`::1`) had no pattern, since the
existing one requires a group before the `::`.
Closes 8 corpus gaps. The punctuation sweep is down to 2 entries, both
leading-dash NANP forms.
* fix(redaction): validate the base58 checksum before redacting a wallet address
The worst precision number in the corpus: a synthetic sample of base58-shaped
opaque ids beginning with `1` or `3` was redacted at a measured 100%, because
the detector was a pure shape match. A customer enabling `crypto_wallet` lost
unrelated identifiers wholesale.
The last four bytes of a base58 Bitcoin address are the first four of the double
SHA-256 of the rest. Checking them takes that rate to 0 on the same sample while
still matching real addresses.
That needs a synchronous digest, and `crypto.subtle` is asynchronous while the
redaction walk is not. `sha256Bytes` in `@repo/utils` is pure TypeScript rather
than `node:crypto`, so the module stays loadable in a browser bundle like the
rest of the package, and it is verified against Web Crypto at every padding
block boundary as well as against the published test vectors.
Ethereum is deliberately left alone. EIP-55 only checksums mixed-case addresses,
and the collision that matters is an all-lowercase 40-character hex string,
which is also exactly the length of a SHA-1 digest. No checksum can separate
those, so it stays a known limit and `crypto_wallet` stays off by default.
* docs(redaction): record measured detector coverage and known limits
Rewrites the §5 detector table against what the detectors now do, and records
that recall was raised above precision along with the evidence for reversing a
stance the spec argued for at length. Adds a Phase 7 task section with the
before and after numbers and the findings worth keeping.
The public page gains a known-limits section. Deterministic detection has edges
and it is more useful to publish them than to let customers find them: numeric
JSON values are never scanned, bare ten-digit phone numbers and nine-digit SSNs
are not matched, national formats with no country code are not matched, a
credential under a key named only `*_KEY` is not matched, and about one in ten
16-digit ids beginning with 4 or 5 collides with card detection.
The per-entity UI copy named `402-118-2260` as a phone false positive, which the
area and exchange code check has since made accurate — it is not redacted any
more. Replaced with a shape that still is, and the card and wallet cautions now
name the collisions that remain rather than ones that do not. Every example was
checked against the detectors rather than written from memory.
* test(redaction): document why the vendor fixtures are assembled from parts
A detector fixture is worthless unless it is shaped exactly like the real thing,
and that shape is what GitHub push protection blocks on: five of these
fabricated values were flagged as Slack, Stripe, SendGrid and GitLab tokens and
a Slack webhook URL.
Joining the parts keeps the value identical at runtime while leaving no
contiguous vendor-shaped literal in the source. That beats allowlisting each one
through the unblock URL, which would bypass the protection permanently for this
repository and leave anyone who forks it hitting the same wall.
* fix(redaction): match bearer auth schemes case-insensitively
`authorization: bearer 9aZq…` stored the token verbatim. RFC 7235 scheme names
are case-insensitive and a lowercase header is common in logs, so the pattern
has to be too.
One consequence worth naming: with the flag, a bare lowercase `token <value>` in
prose also matches. The 16-character minimum and the restricted value charset
keep ordinary sentences out, and the corpus records no new false positive, so
this stays consistent with redacting on ambiguity.
* refactor(redaction): use node:crypto for the address checksum
Replaces the hand-rolled SHA-256 with `createHash`, deleting 94 lines of
hand-written crypto and its verification test.
The standard library does have a synchronous digest; the reason it looked
unavailable is that `@repo/utils` is loadable in a browser bundle and its crypto
module deliberately keeps no static `node:crypto` dependency, while
`crypto.subtle` is async-only and the redaction walk is synchronous. Moving the
helper next to its only caller resolves that: redaction is reached from the
package's server entry alone, `browser.ts` does not export it, so nothing here
ships to a browser. The architecture skill allows a Node API outside the
web-standards-first scope with a comment saying why, which the file now carries.
The checksum itself stays. Decoding to exactly 25 bytes only takes the base58
collision rate from 100% to 11%; the checksum is what takes it to zero.
* style(redaction): cut the detector comments back to traps
The detector comments went from 27 lines to 162 in this branch, roughly a third
of the file, and much of it narrated worked examples, measurements and the order
the fixes happened in. AGENTS.md asks for rare, short comments covering only what
the code cannot show, with rationale in the commit or the spec.
Down to 84 lines. What stays is the traps: guards that look simplifiable and are
not, bounds that are load-bearing rather than decorative, and the two key names
deliberately excluded from credential detection. What went is every measurement,
every before-and-after, and every reference to how a fix was found — all of it
already in the commit history and the spec.
* test(redaction): apply the fixture split to every vendor credential
The comment claimed the file held no contiguous token-shaped literal, but only
the five values GitHub had blocked were split. `AKIA…`, `AIzaSy…`, `ghp_…`,
`hf_…`, `npm_…`, `ya29.…` and `sk-proj-…` were still written whole, and
CodeRabbit's own scanner flagged the Google key among them.
Splitting only what one scanner happens to flag is not a rule, it is a moving
target. All of them go through the same `vendorToken(prefix, body)` helper now,
in the four redaction test files that carry one, so the claim holds
unconditionally and is checkable with a grep.
* docs(redaction): correct three claims the measurements do not support
Three overstatements, all found in review.
The card collision caution gave one rate for two shapes. The probes measure
9.67% for a 16-digit id beginning with 4 and 5.23% for one beginning with 5, so
"about one in ten" was right for 4 and roughly double the truth for 5.
The wallet copy claimed checksum validation for Bitcoin addresses generally.
Only the legacy base58 form is validated; `bc1…` bech32 has no validator and is
a shape match, as is Ethereum. The spec table already said this correctly — the
UI and public docs did not.
The spec said the detectors removed "only 70%" of the corpus while its own
before-numbers gave 60%. The 70% came from the larger corpus I measured with
before trimming it for the repo; against the corpus the spec actually quotes it
is 60%. Both spots now say 60% and the after line states its 118-occurrence
total, which differs from the 117 before because a SendGrid case joined the
corpus in P7-7.
* test(redaction): stop the fixtures reading as card numbers to static analysis
The Slack bot token fixture carried Slack's two numeric segments, and the second
was 13 digits, which is a card length — OpenGrep read it as a possible PAN. One
segment is enough for a fixture whose only job is to satisfy `xox[abposr]-`
followed by ten characters and a digit.
The card comment made the same point with a complete Visa test number in it. The
trap is the trailing period, not the digits, so it reads as prose now.
Neither was blocking any check; both were scanner noise that every future run on
this repository would have reproduced.
* feat(redaction)!: retire the crypto_wallet entity
Per review: nobody has asked for wallet detection, and the entity was never
safe to enable regardless of what Bitcoin did. An all-lowercase 40-character hex
string is both an Ethereum address and a SHA-1 digest, and EIP-55 only checksums
mixed-case addresses, so that half had no fix. The base58 checksum was buying
precision on the one form that was not the problem.
Removes three patterns, the base58 decoder, the checksum, and the digest helper
Andrés and CodeRabbit both objected to — the `node:crypto` question goes away
with the code that needed it.
Retiring an enum member is the risky part, because two things outlive the
deploy: settings rows naming the entity, and queue jobs already carrying it in a
serialized policy.
`wireRedactionEntitiesSchema` drops retired entities and then validates the rest
strictly. The distinction matters and the two cases look identical on the wire. A
retired entity was removed deliberately, so no detector will ever claim it and
dropping it is correct — failing would make the worker fail closed and drop the
batch, which an existing test asserts for malformed policies. An entity that is
neither current nor retired means the policy came from a newer deploy than the
worker, where it may name a detector this code lacks; ignoring that
under-redacts, so it still fails closed. `RETIRED_REDACTION_ENTITIES` is what
tells them apart.
The Privacy page needed the same treatment for a different reason: the form
round-trips the entities it read into a write that validates strictly, so a
project with the retired entity stored would have been unable to save its policy
at all.
Breaking for API clients that send `crypto_wallet` in `settings.redaction.entities`:
the public enum no longer accepts it. Manifests, both SDKs and the CLI bundle are
regenerated.
* chore(cli): regenerate the CLI spec after rebasing onto development
The rebase left `openapi0.json` conflicted, and taking either side by hand was
wrong: it dropped the `429` export-rate-limit response that #4228 added on
development. Regenerating from the emitted spec restores it alongside the
entity-enum change from this branch.
* fix(redaction): stop the file-extension gate shadowing live TLDs
`md`, `py`, `sh` and `zip` were all in `FILE_EXTENSION_TLDS`, and the gate only
applies to two-label domains, so `cliente@empresa.py` and `user@example.md` were
not redacted at all. That is Moldova, Paraguay, Saint Helena and every `.zip`
domain losing email coverage to a list meant to catch `logo@2x.png`.
Removing them costs nothing on the precision side: none of the measured asset-name
false positives ends in one. Where an extension really is a live TLD, redacting a
possible address beats missing a real one.
Audited the whole set rather than taking the reported list — that one included
`sql`, which is not delegated. The remaining entries are checked against the root
zone and the constraint is now written above the set so the next addition gets the
same treatment.
* test(redaction): assemble every credential fixture, not just the prefixed ones
The helper only took a vendor prefix and a body, so the credentials with no
prefix stayed contiguous and kept tripping scanners: an opaque bearer token, a
32-hex vendor key, an AWS secret access key, and two base64 blobs in the
Kubernetes manifest — one of which decoded to a Stripe-shaped key.
`credential(...fragments)` replaces it, which covers both shapes with one idiom
and makes the claim in the file comment true rather than nearly true. The
manifest values are now `btoa` of the split plaintext, so the fixture shows what
it is instead of carrying an opaque encoded blob.
One value changed rather than moved: the manifest password had been the base64 of
`hunter2CorrectHorse`, and it is now the base64 of the `.env` fixture it is
supposed to mirror, `hunter2Correct-Horse`. Both sides of the corpus reference the
same constant, so the case still asserts what it did.
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
2 participants
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.
Throttles
exportTraceson the public API — the one async export endpoint that PR #4137 missed.Rebased onto
development. One file carries the change —packages/operations/src/operations/traces.ts. Of the other 10, five are tool-generated from it (openapi.json, the CLI spec, the TS + Python clients) and five are release bookkeeping (3 CHANGELOGs, 2 version manifests). The facet read-path half of the original commit is gone: Phase 3 (#4199) and Phase 4 (#4204) shipped all of it, and better. Details below, since the original description was Cursor boilerplate.Why this PR exists
Opened by the critical-bug-management automation on 2026-07-26, two days after two related PRs landed:
fix(security): throttle public async exports(also cursor[bot], 2026-07-24) introducedenforceExportRequestRateLimitand wired it intoapps/webdatasets/signals/traces plus@repo/operationsdatasets and signals. It never touchedpackages/operations/src/operations/traces.ts.So the web app's trace export was throttled while the public API's was not. The automation caught that. It also, unprompted, tried to wire
facetIdthrough the scoped behaviour reads; that half raced Phase 3/4 and lost.What ships
The throttle.
exportTracesenqueues an email-delivering CSV export job. Without the throttle an API caller could enqueue unbounded export jobs and unbounded outbound email per (org, project, recipient), whereexportSignalsandexportDatasetRowsare both capped at 10/hour in production. Same call, same placement (after the membership check, beforequeuePublisher.publish), sameEffect.tryPromise+catch: (cause) => causeshape both siblings already use.The documented 429. The original commit added the throttle but left the OpenAPI contract claiming 202/400/401/404 only, so SDK and MCP consumers would hit an undocumented error. Fixing that meant replacing
typedResponseswith the explicitresponsesobject on this one route —typedResponsesdeliberately cannot express extra statuses (the "silent foot-gun" its own docstring describes), and it's exactly whyexportSignalsandexportDatasetRowsuse the object form. Every other route in the file keepstypedResponses.apps/api/openapi.jsonis emitted, not edited;mcp.jsoncame back unchanged (responses aren't part of tool manifests).What I dropped, and why
Everything facet-related. The original commit threaded
facetIdthroughget-cluster-session-intelligence,list-behaviour-sessions,get-behaviour-trajectory,list-project-behaviours, both taxonomy repos, their ports and fakes. All of it is already ondevelopment— 10 of the commit's 16 files conflicted, and every conflict was upstream having done the same thing:developmenttodaySCOPED_CLUSTER_MEMBERSHIPusesfacet_id = {facetId:String}scopeParams(customBehaviorId, facetId)facetIdon the 4 read use-casesfacetIdongetClusterAssignmentCounts/getClusterTrendCountsfacetIdon the fake assignment repoThe two designs differ, and upstream's is the better one, so I took it wholesale:
CustomBehaviorRepository(newresolveScopedBehaviorViewhelper). That adds a Postgres round-trip per read and a new required service to four use-cases that previously needed none — which is whyread-use-cases.test.tsneeded 152 lines of churn to inject a behavior repo into tests that don't care about behaviors.development:facetIdis an explicit optional on each contract, threaded from the client (behaviour-scope.ts→taxonomy.functions.ts→ use-case → repo). No extra read, no new service.Upstream also got a subtlety this commit got wrong.
get-behaviour-trajectory.tsondevelopmentspreads the two ids independently, with a comment explaining why:The dropped version nests
facetIdinside thecustomBehaviorId != nullbranch — the exact failure mode that comment warns about.Net: the whole facet half was redundant, so it's dropped rather than kept as dead code. Nothing about the facets model is left unwired by dropping it.
One thing I did not change, flagging for you
TaxonomyViewAssignmentRepositoryShape.listByBehavioris the last method in that port still hardcodingAND facet_id = ''— every sibling read now takesfacetId. The original commit added the param there too.I left it alone because it has no production callers — only the port, the Live impl, the fake, and its test. Adding a parameter nothing passes is speculative, and if a facet-aware caller ever appears,
facet_id = ''returning topic edges is a loud wrong answer rather than a silent one. If you'd rather it match its siblings now for consistency, it's a four-line change and I'll add it.Client surface
The 429 is a public contract change, so the generated clients ship with it in this PR — CI's
manifestsjob hard-fails on drift inpackages/sdk/{typescript,python}/srcandpackages/cli, which is exactly how it caught this (it was red before the regen).pnpm generate:allproduced the 429 branch inTracesClient.export, both Python raw clients (sync + async), and the CLI's bundled spec. Publishing is version-gated per package, so all three are bumped: SDK manifests to9.7.1, and the CLI via a new## [7.7.1]entry in its CHANGELOG (the CLI has no manifest — that entry is its version, and without it the CLI would regenerate but never ship). Changelog entries added to all three. The file set matches #4212 one-for-one, justtracesin place ofdatasets/signals.Kept as a separate commit (
9b2888d) so the generated diff is skippable — the only hand-written line in it is the version bump.No entry in the root
CHANGELOG.md: that one is written at production-release time from the deploy diff, not per-PR.Verification
pnpm --filter @repo/operations typecheckpnpm --filter @domain/taxonomy typecheckpnpm --filter @app/web typecheckpnpm --filter @platform/db-clickhouse typecheck@domain/taxonomysuitetaxonomy-view-assignment-repository.test.ts(chdb)apps/apiroutes/traces.test.tsPOST /exportenqueue test now running through the throttlepnpm generate:allre-runThe taxonomy checks confirm the conflict resolution left those packages byte-identical to
development; the API route tests are the ones that actually exercise the new code.Review focus
The throttle itself is a three-line copy of an established pattern. Worth your attention instead: (1) do you agree the facet half is fully redundant — that's the judgement call this PR rests on; (2) the
typedResponses→ explicitresponsesswap, since it changes the public contract; (3) whether a patch bump + publish is the right weight for a new documented error code, or whether you'd rather hold the version bumps and let this ride the next SDK release — #4212 set the patch-bump precedent for exactly this and I followed it, but it's your call; (4) thelistByBehaviornote above.