Skip to content

chore(guard): update protobuf#5986

Merged
qw-in merged 7 commits intomainfrom
quinn/update-guard-proto
Apr 8, 2026
Merged

chore(guard): update protobuf#5986
qw-in merged 7 commits intomainfrom
quinn/update-guard-proto

Conversation

@qw-in
Copy link
Copy Markdown
Member

@qw-in qw-in commented Apr 7, 2026

After experimental.1 decided it made sense to break the protobuf. This brings arcjet-js in line. It also drastically improves the jsdoc and fixes the types for custom local rules.

@qw-in qw-in self-assigned this Apr 7, 2026
@qw-in qw-in requested a review from Copilot April 7, 2026 16:24

This comment was marked as outdated.

qw-in added 2 commits April 7, 2026 21:05
…ocs and tests

- Namespace user data under `data:` in defineCustomRule config and input
  to avoid collision with reserved fields (mode, label, metadata)
- Update metadata key constraints docs for capital letter support
- Add default case to reasonFromProto for unknown proto enum values
- Fix async evaluate test to actually be async
- Clarify mixedRuleAllow mock handler doc comment
- Update README getting started to show env var pattern
- Expand test coverage with mixed-rule and multi-custom-rule e2e tests
- Add new mock handlers: mixedRuleAllow, mixedRuleCustomDeny, multiCustomAllow
@qw-in
Copy link
Copy Markdown
Member Author

qw-in commented Apr 8, 2026

Review: chore(guard): update protobuf

Summary

The PR aligns @arcjet/guard with the decide/v2 proto breaking changes from experimental.1, introduces a fully-typed defineCustomRule() API, and improves the public surface (docs, result inspection helpers).


Protobuf Changes

inputKeyinputKeyHash (all rate limit rules)
Correctly renamed in both the generated .js/.d.ts descriptor files and all callers in ruleBodyToProto. Tests verify the new field name.

New configBucket field
Optional on all three rate-limit config types; defaults correctly applied in ruleBodyToProto ("default-token-bucket", "default-fixed-window", "default-sliding-window"). These defaults are applied at proto conversion time, not stored on the config, so rule.config.bucket can be undefined — which is tested and expected.

GuardReason on GuardDecision
The old approach derived reason from the first DENY rule-result case in the SDK. Now the server sends it authoritatively as decision.reason. The new reasonFromProto() is clean, has a default case for future enum values (addressing Copilot's comment), and all 7 values are tested.

errors[] on GuardResponse
hasError() now also returns true when response.errors.length > 0, which is the right place to surface non-fatal validation errors without blocking the decision.


defineCustomRule() API

The three-level call chain (defineCustomRule({evaluate})({data})({data})) separates concerns cleanly:

  1. Define — captures the typed evaluate function once
  2. Configure — creates a module-scope config instance with a stable configId
  3. Submit — creates a per-request input with a fresh inputId

Generic parameters TConfig, TInput, TData flow through end-to-end. The compile-time assertions in custom-rules.test.ts give strong confidence in the type narrowing.

One nit: The evaluate as unknown as CustomEvaluateFn cast is necessary and documented, but note that the internal CustomEvaluateFn type (the untyped one) is still exported. Since defineCustomRule wraps it, consumers shouldn't need the raw type — worth considering whether to un-export it in a follow-up.


AbortSignal Forwarding

Propagated from guard()ruleToProto(rule, signal)ruleBodyToProto(rule, signal)evaluate(config, input, { signal }). The signal === undefined ? {} : { signal } pattern correctly avoids sending signal: undefined in the options object.


Result Inspection Symmetry

The PR adds results() to all RuleWithInput types (single-element or empty array) and result() to all RuleWithConfig types (first result, any conclusion). This makes the API table in the README accurate. One minor observation: results() on a RuleWithInput returning a [T] | [] array is somewhat odd compared to just using result(), but it's useful for iteration symmetry when writing generic code.


Potential Issues

  1. reasonFromProto is called for every decision, including ALLOW. The returned value is only attached to the InternalDecision on DENY paths (correctly), but for ALLOW decisions GuardReason.UNSPECIFIED will return "UNKNOWN" and the value is discarded. This is fine as-is.

  2. multiRuleAllow mock handler uses GuardRuleType.TOKEN_BUCKET for all submissions including custom rules. The new mixedRuleAllow properly handles this case. Tests use the right handler for the right scenario — no issue.

  3. Metadata key constraints were relaxed in the docs (from "lowercase letters" to "ASCII letters"). This should be verified against the actual server-side validation to avoid users hitting unexpected rejections from the API if uppercase isn't actually accepted server-side.

  4. localCustom function is fully removed from public exports ✓. LocalCustomConfig and LocalCustomInput remain as internal types used only within rules.ts.


Documentation

The README is substantially improved. The lifecycle section, anti-pattern (don't wrap launchArcjet() in a helper), bucket guidance, and three-level decision inspection table are all accurate.


Verdict

The implementation is correct and well-tested. All Copilot review comments from CI were resolved. The only actionable item is 3 above (verify server-side acceptance of uppercase metadata keys given the relaxed constraint docs). Everything else is clean.

@qw-in
Copy link
Copy Markdown
Member Author

qw-in commented Apr 8, 2026

verify server-side acceptance of uppercase metadata keys given the relaxed constraint docs

This change is live

@qw-in qw-in marked this pull request as ready for review April 8, 2026 16:07
@qw-in qw-in requested a review from a team as a code owner April 8, 2026 16:07
@qw-in qw-in added this pull request to the merge queue Apr 8, 2026
Merged via the queue into main with commit 25f0e9e Apr 8, 2026
53 checks passed
@qw-in qw-in deleted the quinn/update-guard-proto branch April 8, 2026 16:38
@github-actions github-actions bot mentioned this pull request Apr 8, 2026
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.

3 participants