Add opt-in AURA trust-check adapter (integrations/aura)#10
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds an AURA trust-check adapter: types/constants, auraVerdict network call with timeout and normalization, beforeSettle gating (requireTrust alias) with configurable allow/failOpen semantics, Vitest offline tests, README, threat model, changelog, and a scoped vitest config. ChangesAURA Trust-Check Adapter
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
integrations/aura/adapter.test.tsParsing error: ESLint was configured to run on
integrations/aura/adapter.tsParsing error: ESLint was configured to run on
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…rations/aura/node_modules/.vite/vitest/da39a3ee5e6b4b0d3255bfef95601890afd80709/results.json)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the AURA trust-check adapter, a zero-dependency TypeScript integration for gating settlements based on counterparty reputation. The implementation includes core adapter logic, comprehensive offline tests, a threat model, and documentation. Review feedback focuses on refining the failOpen logic to better handle service unreachability, ensuring the ok property consistently reflects default gating behavior, and improving the clarity and completeness of documentation code examples by addressing missing imports and undefined variables.
| const allow = opts.allow ?? DEFAULT_ALLOW; | ||
| const v = await auraVerdict(did, opts); | ||
| if (allow.includes(v.verdict)) return v; | ||
| if (opts.failOpen && v.verdict === 'unknown' && !v.hasHistory) return v; |
There was a problem hiding this comment.
The current logic for failOpen (opts.failOpen && v.verdict === 'unknown' && !v.hasHistory) might be too broad. It allows a pass not only when the AURA service is unreachable, but also for any 'unknown' verdict where hasHistory is false (like a 'ghost' DID that is known to AURA but has no history). The documentation suggests failOpen should only apply to unreachable cases.
To make this more precise, I suggest adding a dedicated flag to the AuraVerdict object to indicate unreachability. This would make the failOpen logic more robust and aligned with its documented purpose.
This would require the following changes:
- Add
unreachable?: boolean;to theAuraVerdictinterface. - Set
unreachable: falseintoVerdictandunreachable: truein theunreachablehelper function. - Update this line to check
v.unreachableas shown in the suggestion.
| if (opts.failOpen && v.verdict === 'unknown' && !v.hasHistory) return v; | |
| if (opts.failOpen && v.unreachable) return v; |
| import { PayBotClient } from 'paybot-sdk'; | ||
| import { beforeSettle, AuraUntrusted } from './integrations/aura'; | ||
|
|
||
| const client = new PayBotClient(config); | ||
|
|
||
| async function payChecked(counterpartyDid: string, req: PaymentRequest) { |
There was a problem hiding this comment.
The code example in this section has a couple of issues that make it incomplete and potentially confusing for users:
- The
PaymentRequesttype is used inpayCheckedbut is not imported. - The
configobject used to initializePayBotClientis not defined.
To make the example self-contained and easier to follow, please add the missing import and a placeholder for the config object.
| import { PayBotClient } from 'paybot-sdk'; | |
| import { beforeSettle, AuraUntrusted } from './integrations/aura'; | |
| const client = new PayBotClient(config); | |
| async function payChecked(counterpartyDid: string, req: PaymentRequest) { | |
| import { PayBotClient, type PaymentRequest } from 'paybot-sdk'; | |
| import { beforeSettle, AuraUntrusted } from './integrations/aura'; | |
| // Replace with your actual PayBotClient configuration | |
| const config = { apiKey: '...', botId: '...' }; | |
| const client = new PayBotClient(config); | |
| async function payChecked(counterpartyDid: string, req: PaymentRequest) { |
| console.log(v.reason, v.score, v.ok); | ||
|
|
||
| // v.dimensions tells you *which* axis is weak, not just the aggregate: | ||
| if ((v.dimensions?.financial_integrity ?? 1) < 0.4) requireManualReview(); |
There was a problem hiding this comment.
The function requireManualReview() is not defined in this example, which might confuse readers. To improve clarity, please add a comment to indicate that this is a placeholder for the user's own policy logic.
| if ((v.dimensions?.financial_integrity ?? 1) < 0.4) requireManualReview(); | |
| if ((v.dimensions?.financial_integrity ?? 1) < 0.4) requireManualReview(); // Example: your custom policy logic |
| verdict, | ||
| reason: String(body.reason ?? ''), | ||
| score: typeof body.score === 'number' ? body.score : null, | ||
| ok: verdict === 'trusted' || verdict === 'caution', |
There was a problem hiding this comment.
There's a potential for confusion with the ok property. It is true only for 'trusted' and 'caution' verdicts, but the beforeSettle function's default behavior also allows 'new' verdicts. This inconsistency might lead developers to mistakenly believe that v.ok reflects the outcome of beforeSettle.
To improve clarity and consistency, consider aligning the ok property with the default gate behavior by also including 'new'. This would make the ok flag a more reliable indicator of the default gate's pass/fail status.
If you apply this change, please also update the ok column for the new verdict in integrations/aura/README.md.
| ok: verdict === 'trusted' || verdict === 'caution', | |
| ok: ['trusted', 'caution', 'new'].includes(verdict), |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
integrations/aura/adapter.test.ts (1)
122-125: ⚡ Quick winAdd a regression test:
failOpenmust not allow reachableunknownCurrent coverage only verifies transport failure. Add a case where AURA is reachable and returns
unknown(e.g.,did:aura:ghost-bot) withfailOpen: true, and assert rejection withAuraUntrusted.Suggested test addition
describe('network failure', () => { @@ it('gate passes on unreachable when failOpen', async () => { const v = await beforeSettle('did:aura:trusted-bot', { failOpen: true, fetchImpl: failFetch }); expect(v.verdict).toBe('unknown'); }); + + it('failOpen does not pass a reachable unknown verdict', async () => { + await expect( + beforeSettle('did:aura:ghost-bot', { failOpen: true, fetchImpl: okFetch }), + ).rejects.toBeInstanceOf(AuraUntrusted); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/aura/adapter.test.ts` around lines 122 - 125, Add a regression test that verifies failOpen does NOT convert a reachable AURA "unknown" response into a pass: call the same helper used elsewhere (beforeSettle) with a subject that AURA will respond to as unknown (e.g., 'did:aura:ghost-bot') and options including failOpen: true and a fetchImpl that returns an "unknown" response; assert that the call rejects with AuraUntrusted (use expect(...).rejects.toThrow(AuraUntrusted) or equivalent). Target the test in integrations/aura/adapter.test.ts alongside the existing unreachable failure test and reference beforeSettle and AuraUntrusted so the test fails if behavior regresses.integrations/aura/THREAT_MODEL.md (1)
43-52: 💤 Low valueOptional: Add language hint to fenced code block.
The fenced code block at line 43 lacks a language specifier, triggering a markdown linter warning. While the ASCII diagram renders fine, adding a language hint improves markdown validity.
♻️ Proposed fix
## Trust boundary summary -``` +```text PayBot host --(DID only, HTTPS GET)--> AURA /check --> verdict🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/aura/THREAT_MODEL.md` around lines 43 - 52, The fenced ASCII diagram in THREAT_MODEL.md is missing a language hint; update the triple-backtick fence around the diagram (the block containing "PayBot host --(DID only, HTTPS GET)--> AURA /check --> verdict") to include a language specifier such as "text" (i.e. change ``` to ```text) so the markdown linter stops warning while keeping the content unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integrations/aura/adapter.ts`:
- Around line 150-151: The current check allows opts.failOpen to let any v with
verdict === 'unknown' and no history pass, including valid reachable AURA
"unknown" responses; restrict this to only transport-level failures by changing
the condition to require a transport failure indicator (e.g., v.transportFailure
|| v.error || similar flag populated when the AURA call failed to reach the
service) in addition to opts.failOpen, v.verdict === 'unknown' and
!v.hasHistory, and otherwise throw new AuraUntrusted(v); update the code that
builds v if needed to set that transport-failure flag so the adapter only fails
open on transport errors.
In `@integrations/aura/README.md`:
- Around line 17-40: The README uses a relative import for beforeSettle and
AuraUntrusted from './integrations/aura' which isn't exposed by package.json, so
consumers cannot import those symbols; fix by either adding a subpath export for
"./integrations/aura" in package.json pointing to
dist/integrations/aura/index.{js,d.ts}, or re-export beforeSettle and
AuraUntrusted from the package's main entry (index.ts) so users can import them
from 'paybot-sdk', or update the README example to import from the actual public
path your package exposes; ensure the referenced symbols beforeSettle and
AuraUntrusted are exported where you add the subpath or re-export.
---
Nitpick comments:
In `@integrations/aura/adapter.test.ts`:
- Around line 122-125: Add a regression test that verifies failOpen does NOT
convert a reachable AURA "unknown" response into a pass: call the same helper
used elsewhere (beforeSettle) with a subject that AURA will respond to as
unknown (e.g., 'did:aura:ghost-bot') and options including failOpen: true and a
fetchImpl that returns an "unknown" response; assert that the call rejects with
AuraUntrusted (use expect(...).rejects.toThrow(AuraUntrusted) or equivalent).
Target the test in integrations/aura/adapter.test.ts alongside the existing
unreachable failure test and reference beforeSettle and AuraUntrusted so the
test fails if behavior regresses.
In `@integrations/aura/THREAT_MODEL.md`:
- Around line 43-52: The fenced ASCII diagram in THREAT_MODEL.md is missing a
language hint; update the triple-backtick fence around the diagram (the block
containing "PayBot host --(DID only, HTTPS GET)--> AURA /check --> verdict") to
include a language specifier such as "text" (i.e. change ``` to ```text) so the
markdown linter stops warning while keeping the content unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1192a76-e834-4ef9-bae1-99a571457255
⛔ Files ignored due to path filters (1)
integrations/aura/node_modules/.vite/vitest/da39a3ee5e6b4b0d3255bfef95601890afd80709/results.jsonis excluded by!**/node_modules/**
📒 Files selected for processing (6)
integrations/aura/README.mdintegrations/aura/THREAT_MODEL.mdintegrations/aura/adapter.test.tsintegrations/aura/adapter.tsintegrations/aura/index.tsintegrations/aura/vitest.config.ts
| ```ts | ||
| import { PayBotClient } from 'paybot-sdk'; | ||
| import { beforeSettle, AuraUntrusted } from './integrations/aura'; | ||
|
|
||
| const client = new PayBotClient(config); | ||
|
|
||
| async function payChecked(counterpartyDid: string, req: PaymentRequest) { | ||
| try { | ||
| await beforeSettle(counterpartyDid); // rejects high_risk + unknown | ||
| } catch (e) { | ||
| if (e instanceof AuraUntrusted) { | ||
| console.warn('blocked:', e.message); // your policy decides | ||
| return; | ||
| } | ||
| throw e; | ||
| } | ||
| return client.pay(req); // existing flow, unchanged | ||
| } | ||
| ``` | ||
|
|
||
| `payTo` in a `PaymentRequest` is a wallet address; the AURA DID is the | ||
| counterparty's portable identity, supplied by your own mapping. The gate keys | ||
| on that DID — it composes cleanly with PayBot's existing `TRUST_VIOLATION` | ||
| error model as a *pre-flight* reputation axis. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if package.json has subpath exports configured for integrations/aura
if [ -f package.json ]; then
echo "=== Checking package.json exports configuration ==="
jq -r '.exports // "No exports field"' package.json
else
echo "No package.json found at root"
fi
# Check for any existing integration imports in the codebase
echo -e "\n=== Searching for existing integration import patterns ==="
rg -n --type=ts "from ['\"].*integrations/" -C 2 || echo "No existing integration imports found"Repository: RBKunnela/paybot-sdk
Length of output: 570
Fix import path: package.json lacks subpath exports for the documented example.
The README example (line 19) shows import { beforeSettle, AuraUntrusted } from './integrations/aura', but package.json currently exports only the root entry point. SDK consumers cannot use the relative import path as documented.
Either:
- Add subpath export to package.json:
"./integrations/aura": { "import": "./dist/integrations/aura/index.js", "types": "./dist/integrations/aura/index.d.ts" } - Or re-export from the main index.ts so users import from
'paybot-sdk' - Or update the example to match the actual consumable import path
Confirm which pattern aligns with your SDK's public API design.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integrations/aura/README.md` around lines 17 - 40, The README uses a relative
import for beforeSettle and AuraUntrusted from './integrations/aura' which isn't
exposed by package.json, so consumers cannot import those symbols; fix by either
adding a subpath export for "./integrations/aura" in package.json pointing to
dist/integrations/aura/index.{js,d.ts}, or re-export beforeSettle and
AuraUntrusted from the package's main entry (index.ts) so users can import them
from 'paybot-sdk', or update the README example to import from the actual public
path your package exposes; ensure the referenced symbols beforeSettle and
AuraUntrusted are exported where you add the subpath or re-export.
|
Thanks @coderabbitai @gemini-code-assist — good catch on |
|
Thanks for the update, @luisllaver. The distinction between transport failures and reachable |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 16584
The test coverage hits both One small doc inconsistency: the README still says "17 tests" at the bottom, but you're now at 19. Worth a one-line fix before merge. Everything else looks clean — the |
RBKunnela
added
the
friendlyai-review-refresh
|
@luisllaver — apologies for the silence here. PR #10 has been sitting at CHANGES_REQUESTED since 2026-05-21 while we hardened the security perimeter on the repo (FriendlyAI review, OSV-Scanner, CodeQL, Dependabot batching, CodeRabbit autofix, auto-merge — most of it just shipped). Your PR was already in good shape and got buried under that work. That's on me, not you. Where this stands now: Reading back through gemini-code-assist's and CodeRabbit's reviews, the substantive feedback was already addressed in your last push:
What's still blocking the CHANGES_REQUESTED label is, as far as I can see, a stale review state — not a missing change. The PR description matches what's in the diff. Next step — pick one:
I want to keep the boundary we agreed on earlier in #3: opt-in adapter, no hard dependency on AURA from the SDK core. Your implementation respects that. I'm ready to merge when you are. Also — thank you for being patient. The bot-native trust gating you're building is exactly the kind of thing this protocol needs. — Renata (RBKunnela) |
|
Update — full transparency on the CI workflow situation (2026-05-25) Adding context that explains the lag you've been seeing: We have four review agents running on every PR (CodeRabbit, gemini-code-assist, FriendlyAI review gate, CodeQL/OSV scanners). The FriendlyAI review gate is the unstable one — it's a wrapper around a stack of internal agents from a private repo that I don't want to expose. The wrapper itself is what we've been hardening this week (PRs #44, #45, #46 in paybot-sdk; PR #34 in paybot-mcp), and during that hardening pass the queue behavior was inconsistent: PRs landed in BLOCKED state, status checks didn't fire on rebase, and the queue silently dropped some events. That's the root cause of the silence on this PR. Not a quality issue with your work. Not a change in scope. The pipeline itself was unreliable while I rebuilt it. We are still open to contributors. Genuinely. Every bot, every human-with-bot, every solo builder who has shown up to this protocol — you are part of why this matters. The receipt primitives, the trust adapters, the Python parity, the SDK surface feedback — none of it could have come from one person. I'm asking for patience while the wrapper stabilizes. If your PR is sitting in a weird state, ping me here or on issue #3 and I will manually walk it through. No PR gets dropped silently again. That's the commitment. — Renata (RBKunnela), via aion-rbk |
|
@RBKunnela no worries at all — appreciate the transparency on the CI rebuild, and glad the protocol's still open to contributors. Confirmed on my side: nothing outstanding from the gemini/CodeRabbit threads — the Pinging for re-review as you suggested — happy for you to dismiss the stale state and land it. If the status checks need a fresh trigger (the PR shows mergeable but |
|
Separate from the merge mechanics - once it's in and you've had agents actually run against it, I'd genuinely value your honest read:
We build this to be used, so candid feedback - including what's missing or annoying - is worth more to us than the merge itself. No rush; whenever you've kicked the tires. Thanks for taking it seriously through the CI rebuild. |
|
Quick status from my side: no check runs fired on the latest commit ( |
RBKunnela
dismissed
coderabbitai[bot]’s stale review
Nits-only review (2 items: README failOpen example transport-flag clarification + THREAT_MODEL.md fenced-block lang hint). @qa cross-checked with GLM-4.6 — non-blocking. Tracked in follow-up doc-nit issue per automated-pr-merge-authority chain.
RBKunnela
added
the
friendlyai-bypass-ack-by-maintainer
…stead of failure) (#52) GitHub Actions does not pass repository secrets to workflows triggered by `pull_request` from forks (security policy — prevents secret exfil via malicious PR payloads). The current single-job workflow runs the underlying action regardless of fork status, and the action fails with `Input required and not supplied: review-api-key` when `secrets.FRIENDLYAI_REVIEW_API_KEY` is empty. That fails the required `friendlyai-review verdict` check and permanently blocks the PR. Split the workflow into two mutually-exclusive jobs sharing the same check name: - `review` runs only when head.repo == base.repo (same-repo PR, secrets available) and invokes the FriendlyAI Review action as before. - `review-fork` runs only when head.repo != base.repo (fork PR, no secrets) and posts a NEUTRAL check-run named `friendlyai-review verdict` plus an idempotent sticky comment explaining the situation. Neutral conclusion satisfies branch-protection required-status-checks non-blockingly. A maintainer can still get a full verdict by mirroring the fork branch to a maintainer-owned branch (which CAN access secrets). Live example: luisllaver/paybot-sdk PR #10 is blocked by exactly this issue and will be unblocked by this change. Both `actions/github-script` invocations are SHA-pinned to the v8 release commit (`ed597411d8f924073f98dfc5c65a23a2325f34cd`) — floating `@v8` tag fails supply-chain hygiene. `pull-requests` permission upgraded from `read` to `write` so the fork-branch sticky comment can be posted. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the AURA trust-check adapter, @luisllaver — appreciated. 🙏 I ran the full gate against your branch locally; here's exactly where it stands so you can get it green: ✅ What's good
🔧 What needs fixing before review
Once it's rebased and the adapter has tests, ping here and we'll review for merge. Solid contribution — just needs to catch up to — @devops |
|
Closing to keep the queue clean — and this is not a 'no', @luisllaver. Your AURA adapter's own tests pass; the blockers are mechanical: the branch is well behind |
2 participants
Opt-in, read-only AURA trust-check adapter — picking up the Founding Contributors invite (#3).
What it is — a
integrations/auramodule: abeforeSettle(did)gate to run in front ofclient.pay(). Backward-looking reputation as a pre-flight axis beside PayBot's ownTRUST_VIOLATIONcheck — kept separate so the settle decision stays auditable.API
auraVerdict(did)→ typed{verdict, reason, score, ...}(trusted|caution|high_risk|new|unknown)beforeSettle(did)/requireTrust(did)→ throwsAuraUntrustedon high_risk + unknownGET /check?did=...(zero-auth). Does not sign, hold keys, or move USDC.Boundary — zero deps (global fetch, Node 18+). Off by default. Fail-closed by default; set
failOpen: trueto let an unreachable AURA pass. Purely additive — remove it and PayBot is unchanged.Tests —
npx vitest run --config integrations/aura/vitest.config.ts→ 17 offline tests via afetchImplinjection seam: every verdict class, the gate's allow-list + failOpen, the unreachablepath, input validation. Strict
tscclean.See
integrations/aura/README.md(wrapsclient.pay()) andTHREAT_MODEL.md.Opened as draft — happy to align naming/paths with paybot-sdk conventions before you merge.
Summary by CodeRabbit
New Features
Documentation
Tests