Inbound FEP-044f quote authorization#31
Conversation
Add FEP-044f quote policies to published messages and handle incoming QuoteRequest activities. Accepted quote requests now issue and store QuoteAuthorization stamps, and bot applications can inspect or moderate requests through the new onQuoteRequest handler. Persist quote authorization stamps across the memory, SQLite, and PostgreSQL repositories. Keep duplicate requests idempotent by preserving the first stamp for a quote post, and allow later revocation by quote URL even when the remote quote can no longer be fetched. Document the quote policy and revocation APIs, and record the change for the PR expected to carry this work. fedify-dev#27 Fixes fedify-dev#28 fedify-dev#31 Assisted-by: Codex:gpt-5.5
|
@codex review |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds quote policy configuration, inbound quote request handling, quote authorization storage, revocation support, and related API, persistence, test, and documentation updates across BotKit. ChangesQuote Request and Authorization Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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 inbound support for consent-respecting quote posts using FEP-044f. It adds quote policies on outgoing messages, handles incoming QuoteRequest activities, automatically accepts or rejects them according to each message's policy, and stores QuoteAuthorization stamps for accepted quotes. It also updates the database schemas for SQLite and Postgres repositories to support quote authorizations, adds documentation, and includes comprehensive unit tests. There are no review comments to address, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![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: 43728f3014
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/botkit-postgres/src/mod.ts (1)
132-168: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAdvisory lock needs an actual transaction.
initializePostgresRepositorySchemaaccepts a plainQueryableand skipssql.begin()when.begin()is absent, sopg_advisory_xact_lockis released after each statement and does not serialize the schema DDL in that branch. Require a transactional queryable here, or switch the fallback path topg_advisory_lock/pg_advisory_unlock.🤖 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 `@packages/botkit-postgres/src/mod.ts` around lines 132 - 168, The advisory lock in initializePostgresRepositorySchemaInTransaction only works for the duration of a real transaction, but the current fallback path lets plain Queryable callers bypass sql.begin() and lose serialization. Update initializePostgresRepositorySchema so it either requires a transactional Queryable/always runs inside sql.begin(), or change the non-transactional branch to use a session-scoped lock pair instead of pg_advisory_xact_lock. Keep the fix centered on initializePostgresRepositorySchema and initializePostgresRepositorySchemaInTransaction.
🧹 Nitpick comments (7)
packages/botkit/src/quote.ts (1)
143-159: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAsymmetry between explicit
"nobody"and an unset axis for self-approval.For
"nobody"withautomatic: true, the actor itself is always included ([actorUri]), but when the automatic axis is simply unset (undefined, e.g. a{ manual: "followers" }policy), the default branch returns[]— omitting self entirely fromautomaticApprovals. This is inconsistent, though low-impact since inbound handling special-cases the bot's own actor separately (see#matchesQuoteAcceptanceinbot-impl.ts).🤖 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 `@packages/botkit/src/quote.ts` around lines 143 - 159, The serialization logic in serializeQuoteAcceptance is inconsistent for self-approval: the explicit "nobody" case includes actorUri when automatic is true, but the unset/default path returns an empty list. Update serializeQuoteAcceptance so the default branch matches the intended automatic self-approval behavior for unset/manual policies, keeping the treatment consistent with the "nobody" branch and the automaticApprovals handling used by quote.ts.packages/botkit/src/quote.test.ts (1)
46-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding coverage for default policy and combined automatic+manual serialization.
Missing test cases:
serializeQuotePolicy(undefined, ...)(default "public"),"nobody"for the manual axis, and an object policy with bothautomaticandmanualset together.🤖 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 `@packages/botkit/src/quote.test.ts` around lines 46 - 77, Add test coverage in serializeQuotePolicy() for the missing policy shapes: verify serializeQuotePolicy(undefined, actor, followers) defaults to the public automatic approvals, add a case for the manual axis with "nobody", and add a combined object policy that sets both automatic and manual to confirm both approval arrays serialize correctly. Update the tests in quote.test.ts by extending the existing serializeQuotePolicy() describe block and using the existing actor/followers fixtures and canQuote assertions.packages/botkit/src/bot.ts (1)
191-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDoc wording inconsistent with
CreateBotOptions.quotePolicy/BotProfile.quotePolicy.This doc only describes incoming-request handling, but per
session-impl.ts(options.quotePolicy ?? this.bot.quotePolicy) this property is also the default policy advertised on outgoing published messages.CreateBotOptions.quotePolicy(Line 367-376) andBotProfile.quotePolicycorrectly describe both roles ("advertised on outgoing messages" + "fallback ..."). Aligning the wording here avoids confusing API consumers about the dual purpose.📝 Proposed doc alignment
/** - * How the bot handles incoming quote requests for its messages. + * Who can quote messages published by the bot. + * + * This policy is advertised on outgoing messages and is used as the + * fallback for older messages that do not have a serialized quote policy, + * as well as for evaluating incoming quote requests when the target + * message has no serialized policy of its own. * `@since` 0.5.0 */ readonly quotePolicy: QuotePolicyOption;🤖 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 `@packages/botkit/src/bot.ts` around lines 191 - 196, Update the `quotePolicy` doc on `BotProfile` in `bot.ts` so it matches `CreateBotOptions.quotePolicy` and `BotProfile.quotePolicy` usage elsewhere: describe that this policy is both the default advertised on outgoing published messages and the fallback for handling incoming quote requests. Keep the change limited to the documentation comment near `quotePolicy`, and align the wording with the behavior implied by `session-impl.ts` (`options.quotePolicy ?? this.bot.quotePolicy`).packages/botkit/src/message.ts (2)
260-264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the omitted-
quotePolicybehavior on update.Per
message-impl.ts(options.quotePolicy == null ? message.interactionPolicy : serializeQuotePolicy(...)), omittingquotePolicyonupdate()preserves the message's existing policy — it does not reset to the bot's default the way an omittedquotePolicydoes inSessionPublishOptions(Line 197-203 ofsession.ts). This asymmetry is easy to miss and worth spelling out explicitly.📝 Proposed doc clarification
export interface AuthorizedMessageUpdateOptions { /** * Who can quote the updated message. + * + * If omitted, the message's existing quote policy is preserved unchanged. */ readonly quotePolicy?: QuotePolicyOption; }🤖 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 `@packages/botkit/src/message.ts` around lines 260 - 264, Document the omitted-quotePolicy behavior for AuthorizedMessageUpdateOptions so it is clear in update() that leaving quotePolicy unset preserves the message’s existing interactionPolicy rather than resetting to the bot default. Add this clarification to the quotePolicy field docs in message.ts, and align the wording with the update flow in message-impl.ts where options.quotePolicy == null falls back to message.interactionPolicy; note the contrast with SessionPublishOptions in session.ts if needed.
226-254: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
update/unauthorizeQuotedon't accept anAbortSignal.Both are new/modified async public APIs, but the coding guidelines for
**/*.{ts,tsx}require async functions to accept anAbortSignalfor cancellation. That said, this matches the pre-existing convention across the rest ofSession/AuthorizedMessage(e.g.follow,unfollow,publish,delete), none of which accept one either, so fixing it in isolation here would be inconsistent with the broader API surface.As per coding guidelines: "All async functions must accept an
AbortSignalparameter to support cancellation."🤖 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 `@packages/botkit/src/message.ts` around lines 226 - 254, `AuthorizedMessage.update` and `AuthorizedMessage.unauthorizeQuote` are async public methods that currently do not accept an `AbortSignal`. Update their signatures (and the corresponding implementations/call sites in `message.ts` and any concrete classes using these methods) to include an optional cancellation signal and propagate it through the underlying async work, keeping the API consistent with the project’s async cancellation guideline and existing symbols like `AuthorizedMessage`, `update`, and `unauthorizeQuote`.Source: Coding guidelines
packages/botkit/src/repository.test.ts (1)
60-135: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider adding a concurrent-duplicate test for
KvRepository.All new tests call
addQuoteAuthorizationsequentially. Once the TOCTOU issue flagged inrepository.ts(KvRepository's add/remove reverse-index handling) is fixed, a test firing twoaddQuoteAuthorizationcalls concurrently (Promise.all) for the sameinteractingObjectwould guard against regressions.🤖 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 `@packages/botkit/src/repository.test.ts` around lines 60 - 135, The current quote-authorization tests only cover sequential calls, so they don’t protect against the KvRepository TOCTOU/reverse-index race in addQuoteAuthorization. Add a new concurrent-duplicate test alongside the existing repository factory cases that uses Promise.all to call addQuoteAuthorization twice for the same interactingObject, then assert the repository keeps only the first authorization via findQuoteAuthorization and that the duplicate is not stored. Use the existing addQuoteAuthorization, findQuoteAuthorization, and KvRepository test setup to locate the right spot.packages/botkit/src/bot-impl.test.ts (1)
3341-3346: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
assert.ok(...)for the booleanstartsWithcheck.
startsWith(...)returns a boolean, so comparing it totrueviadeepStrictEqualshould be anassert.ok(...).♻️ Proposed change
- assert.deepStrictEqual( - activity.resultId?.href.startsWith( - "https://example.com/ap/actor/bot/quote-authorization/", - ), - true, - ); + assert.ok( + activity.resultId?.href.startsWith( + "https://example.com/ap/actor/bot/quote-authorization/", + ), + );As per coding guidelines: "avoid
assert.equal(..., true)orassert.equal(..., false)in favor ofassert.ok(...)/assert.ok(!...)."🤖 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 `@packages/botkit/src/bot-impl.test.ts` around lines 3341 - 3346, The boolean prefix check in the test currently uses assert.deepStrictEqual with startsWith(...), which should be simplified to assert.ok because startsWith returns a boolean. Update the assertion in the bot-impl test near activity.resultId?.href.startsWith(...) to use assert.ok(...) directly, keeping the same quoted URL prefix and preserving the existing test intent.Source: Coding guidelines
🤖 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 `@CHANGES.md`:
- Around line 50-71: Normalize the new changelog bullets in CHANGES.md so every
added item uses standalone issue/PR markers in the form [[`#123`]]. Update the
top-level quote-post entry and each nested bullet in the section introduced by
the new quote support changes, replacing bundled markers like [[`#27`], [`#28`],
[`#31`]] with individual tokens and adding markers to any bullet that currently
lacks them. Use the existing changelog bullet text and the quote-related entries
(QuotePolicy, QuoteRequest, Bot.onQuoteRequest,
AuthorizedMessage.unauthorizeQuote, etc.) to locate all affected items.
In `@packages/botkit/src/quote.ts`:
- Around line 119-141: `serializeQuotePolicy` is exported but missing the
required JSDoc documentation. Add a JSDoc block above `serializeQuotePolicy`
that describes its purpose, documents the `policy`, `actorUri`, and
`followersUri` parameters, explains the `InteractionPolicy` return value, and
includes the appropriate `@since` tag to match the other exported APIs in
`quote.ts`.
- Around line 92-103: `QuoteRequest.accept()` and `QuoteRequest.reject()` are
async public methods but currently have no cancellation support. Update the
`QuoteRequest` interface to accept an `AbortSignal` parameter for both methods,
then propagate that change through the implementing `QuoteRequestImpl` methods
and any call sites so the signature stays consistent and cancellable.
In `@packages/botkit/src/repository.ts`:
- Around line 1563-1586: Make addQuoteAuthorization atomic because the current
get/set sequence can race and corrupt the reverse index for the same
interactingObjectId. Update the QuoteAuthorization write path in repository.ts
to follow the same compare-and-swap retry approach used by `#addToFolloweeIndex`,
so only one writer succeeds and the index stays consistent. Keep the fix within
addQuoteAuthorization and its use of this.kv and
`#quoteAuthorizationIndexKey/`#key, and ensure removeQuoteAuthorization won’t be
able to orphan a valid record after concurrent writes.
---
Outside diff comments:
In `@packages/botkit-postgres/src/mod.ts`:
- Around line 132-168: The advisory lock in
initializePostgresRepositorySchemaInTransaction only works for the duration of a
real transaction, but the current fallback path lets plain Queryable callers
bypass sql.begin() and lose serialization. Update
initializePostgresRepositorySchema so it either requires a transactional
Queryable/always runs inside sql.begin(), or change the non-transactional branch
to use a session-scoped lock pair instead of pg_advisory_xact_lock. Keep the fix
centered on initializePostgresRepositorySchema and
initializePostgresRepositorySchemaInTransaction.
---
Nitpick comments:
In `@packages/botkit/src/bot-impl.test.ts`:
- Around line 3341-3346: The boolean prefix check in the test currently uses
assert.deepStrictEqual with startsWith(...), which should be simplified to
assert.ok because startsWith returns a boolean. Update the assertion in the
bot-impl test near activity.resultId?.href.startsWith(...) to use assert.ok(...)
directly, keeping the same quoted URL prefix and preserving the existing test
intent.
In `@packages/botkit/src/bot.ts`:
- Around line 191-196: Update the `quotePolicy` doc on `BotProfile` in `bot.ts`
so it matches `CreateBotOptions.quotePolicy` and `BotProfile.quotePolicy` usage
elsewhere: describe that this policy is both the default advertised on outgoing
published messages and the fallback for handling incoming quote requests. Keep
the change limited to the documentation comment near `quotePolicy`, and align
the wording with the behavior implied by `session-impl.ts` (`options.quotePolicy
?? this.bot.quotePolicy`).
In `@packages/botkit/src/message.ts`:
- Around line 260-264: Document the omitted-quotePolicy behavior for
AuthorizedMessageUpdateOptions so it is clear in update() that leaving
quotePolicy unset preserves the message’s existing interactionPolicy rather than
resetting to the bot default. Add this clarification to the quotePolicy field
docs in message.ts, and align the wording with the update flow in
message-impl.ts where options.quotePolicy == null falls back to
message.interactionPolicy; note the contrast with SessionPublishOptions in
session.ts if needed.
- Around line 226-254: `AuthorizedMessage.update` and
`AuthorizedMessage.unauthorizeQuote` are async public methods that currently do
not accept an `AbortSignal`. Update their signatures (and the corresponding
implementations/call sites in `message.ts` and any concrete classes using these
methods) to include an optional cancellation signal and propagate it through the
underlying async work, keeping the API consistent with the project’s async
cancellation guideline and existing symbols like `AuthorizedMessage`, `update`,
and `unauthorizeQuote`.
In `@packages/botkit/src/quote.test.ts`:
- Around line 46-77: Add test coverage in serializeQuotePolicy() for the missing
policy shapes: verify serializeQuotePolicy(undefined, actor, followers) defaults
to the public automatic approvals, add a case for the manual axis with "nobody",
and add a combined object policy that sets both automatic and manual to confirm
both approval arrays serialize correctly. Update the tests in quote.test.ts by
extending the existing serializeQuotePolicy() describe block and using the
existing actor/followers fixtures and canQuote assertions.
In `@packages/botkit/src/quote.ts`:
- Around line 143-159: The serialization logic in serializeQuoteAcceptance is
inconsistent for self-approval: the explicit "nobody" case includes actorUri
when automatic is true, but the unset/default path returns an empty list. Update
serializeQuoteAcceptance so the default branch matches the intended automatic
self-approval behavior for unset/manual policies, keeping the treatment
consistent with the "nobody" branch and the automaticApprovals handling used by
quote.ts.
In `@packages/botkit/src/repository.test.ts`:
- Around line 60-135: The current quote-authorization tests only cover
sequential calls, so they don’t protect against the KvRepository
TOCTOU/reverse-index race in addQuoteAuthorization. Add a new
concurrent-duplicate test alongside the existing repository factory cases that
uses Promise.all to call addQuoteAuthorization twice for the same
interactingObject, then assert the repository keeps only the first authorization
via findQuoteAuthorization and that the duplicate is not stored. Use the
existing addQuoteAuthorization, findQuoteAuthorization, and KvRepository test
setup to locate the right spot.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 95f9e1d1-260b-4441-93ec-eb65e9714b9d
⛔ Files ignored due to path filters (1)
deno.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
CHANGES.mddocs/concepts/bot.mddocs/concepts/events.mddocs/concepts/message.mdpackages/botkit-postgres/src/mod.test.tspackages/botkit-postgres/src/mod.tspackages/botkit-sqlite/src/mod.test.tspackages/botkit-sqlite/src/mod.tspackages/botkit/deno.jsonpackages/botkit/package.jsonpackages/botkit/src/bot-group.test.tspackages/botkit/src/bot-impl.test.tspackages/botkit/src/bot-impl.tspackages/botkit/src/bot.tspackages/botkit/src/events.tspackages/botkit/src/instance-impl.tspackages/botkit/src/instance.tspackages/botkit/src/message-impl.test.tspackages/botkit/src/message-impl.tspackages/botkit/src/message.tspackages/botkit/src/mod.tspackages/botkit/src/quote-impl.tspackages/botkit/src/quote.test.tspackages/botkit/src/quote.tspackages/botkit/src/repository.test.tspackages/botkit/src/repository.tspackages/botkit/src/session-impl.test.tspackages/botkit/src/session-impl.tspackages/botkit/src/session.tspackages/botkit/src/text.test.ts
Run PostgreSQL schema initialization only through clients that can open a transaction, so the advisory transaction lock covers the schema DDL it is meant to serialize. This removes the fallback path where a plain Queryable could acquire pg_advisory_xact_lock for only one statement and then continue with unlocked DDL. Adjust the schema initialization rejection test to use a transaction-capable fake client, preserving the unhandled rejection coverage under the stricter initializer contract. fedify-dev#31 (review) Assisted-by: Codex:gpt-5.5
Quote authorization should not bypass the audience of the message being quoted. Check the target object's recipients before policy evaluation so hidden messages cannot issue stamps to actors outside their audience. fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
Duplicate QuoteRequest delivery can race the reverse index in KV-backed repositories. Use the same quote-specific lock around insertion and removal so only one stamp is stored for a quote URI and stale removals cannot clear a newer index entry. fedify-dev#31 (comment) fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
Expose cancellation on QuoteRequest accept and reject so handlers can thread abort state through moderation work. Also document the exported policy serializer and keep the changelog's issue markers in the repository style. fedify-dev#31 (comment) fedify-dev#31 (comment) fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces inbound support for consent-respecting quote posts using FEP-044f. It adds new types and options for configuring quote policies, introduces the Bot.onQuoteRequest event handler, and implements the AuthorizedMessage.unauthorizeQuote() method to revoke quote authorizations. Additionally, it updates the SQLite and PostgreSQL repository implementations to support a new quote_authorizations table and corresponding storage methods. I have no feedback to provide as there are no review comments to assess.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12bb347737
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Quote authorization should not let an allowed requester publish a quote to a broader audience than the target message. Compare the computed quote and target visibility before approval, and expose the update options type through the package root so the new update API can be named by callers. fedify-dev#31 (comment) fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces inbound support for consent-respecting quote posts using FEP-044f, adding new quote policy options, handling incoming QuoteRequest activities, and storing QuoteAuthorization stamps across SQLite and PostgreSQL repositories. Feedback on the implementation highlights a potential privacy leak where "unknown" message visibility is not handled conservatively when checking for wider quote audiences. Additionally, defensive checks are recommended to ensure quoteId is not nullish in unauthorizeQuote and that target.id is validated in the QuoteRequestImpl constructor to prevent invalid authorization stamps.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/botkit/src/bot-impl.test.ts`:
- Line 3482: The test in bot-impl.test.ts is asserting a boolean with
deepStrictEqual against false; update the handled assertion to use
assert.ok(!handled) instead. Locate the existing handled check in the relevant
test case and replace the equality-style boolean assertion with a truthiness
check that matches the coding guidelines.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e44da9d2-00ad-41bd-a112-aa7afda7979b
📒 Files selected for processing (10)
CHANGES.mdpackages/botkit-postgres/src/mod.test.tspackages/botkit-postgres/src/mod.tspackages/botkit/src/bot-impl.test.tspackages/botkit/src/bot-impl.tspackages/botkit/src/mod.tspackages/botkit/src/quote-impl.tspackages/botkit/src/quote.tspackages/botkit/src/repository.test.tspackages/botkit/src/repository.ts
✅ Files skipped from review due to trivial changes (1)
- CHANGES.md
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/botkit-postgres/src/mod.test.ts
- packages/botkit/src/mod.ts
- packages/botkit/src/quote.ts
- packages/botkit/src/quote-impl.ts
- packages/botkit/src/repository.ts
- packages/botkit-postgres/src/mod.ts
- packages/botkit/src/bot-impl.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5ef282cc8
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Unknown quote audiences should not receive authorization stamps for restricted targets. Reject those cases before policy approval, keep the boolean assertion style aligned with the test guidelines, and add defensive ID checks before quote authorization lookup or stamp creation. fedify-dev#31 (comment) fedify-dev#31 (comment) fedify-dev#31 (comment) fedify-dev#31 (comment) fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces inbound support for consent-respecting quote posts using [FEP-044f] in BotKit, adding quote policies, QuoteRequest handling, and database support for storing quote authorizations in SQLite and PostgreSQL. The review feedback suggests several defensive checks to prevent potential TypeErrors (such as checking for null request.id, instrument.id, and sql in hasTransaction), a security improvement to silently ignore unauthorized quote requests to prevent information exposure, and forwarding the signal parameter to sendActivity calls in QuoteRequestImpl.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2411c63db4
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Quote request handling now drops malformed requests before building the public request wrapper, avoids leaking restricted target existence, and removes a stale quote stamp when an already-authorized quote is redelivered with a wider audience. The quote request accept path also refuses to reuse an authorization that belongs to another target, and the Postgres transaction guard now tolerates nullish callers defensively. fedify-dev#31 (comment) fedify-dev#31 (comment) fedify-dev#31 (comment) fedify-dev#31 (comment) fedify-dev#31 (comment) fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8735b9865c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Inbound quote handling now selects the FEP quote target before any legacy fallback link. That keeps authorization checks, event routing, and the materialized quote target aligned when a Create carries both forms but they disagree. fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59e1e31ff4
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Inbound FEP quote delivery now re-runs the same audience widening check used for quote requests before dispatching onQuote. This keeps a reused stamp from authorizing a Create whose audience is broader than the quoted target, and drops the stale stamp when that happens. fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6858ba4bac
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
KV quote authorization inserts now verify that an existing quote index still resolves before treating it as authoritative. If the indexed stamp disappeared first, the stale index is cleared and the new stamp is stored so Accept.result can reference a dispatchable authorization. fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecd35ca19f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Inbound FEP quote Creates for local targets now stop before generic message delivery when their QuoteAuthorization is missing or invalid. This keeps pure FEP quote routing from exposing an unauthorized quote to onMessage after onQuote correctly declines it. fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds inbound support for consent-respecting quote posts using FEP-044f. It introduces new types, configuration options, and event handlers for managing quote policies, alongside methods to authorize and revoke quote permissions. It also updates the SQLite and PostgreSQL repository backends with a new quote_authorizations table to store authorization stamps. As there are no review comments, no additional feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3aba8619f2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Code Review
This pull request introduces inbound support for consent-respecting quote posts using FEP-044f, adding new quote policy configurations, event handlers, and database tables for quote authorizations across PostgreSQL, SQLite, and KV/Memory repositories. A potential issue was identified in MemoryRepository.removeQuoteAuthorization where the index entry for quoteAuthorizationsByInteractingObject is deleted without verifying if the indexed ID matches the ID being removed, which could lead to incorrect index deletions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Inbound Update activities that add a FEP quote authorization now route to the quoted bot and reuse the same validation path as Create delivery. The validation path also revokes widened quote stamps through the public revocation flow so Delete activities are sent, and MemoryRepository now keeps reverse indexes unless they still point at the removed stamp. fedify-dev#31 (comment) fedify-dev#31 (comment) fedify-dev#31 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces inbound support for consent-respecting quote posts using FEP-044f. It adds quote policy configurations, a new Bot.onQuoteRequest event handler for manual moderation, and updates both SQLite and PostgreSQL repositories with a new quote_authorizations table to store authorization stamps. It also implements automatic acceptance/rejection logic and the ability to revoke authorizations. The feedback suggests forwarding the AbortSignal to the sendActivity calls in both the accept and reject methods of QuoteRequestImpl to ensure that network requests can be aborted if cancelled.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
This implements the inbound side of FEP-044f for BotKit by treating quote authorization as part of the message lifecycle, rather than as a separate callback-only feature. Outgoing messages now carry an interaction policy, so a remote server can tell whether a quote request should be automatic, manual, or rejected before it asks. Incoming
QuoteRequestactivities then use the stored message policy as the source of truth, with the bot-level policy kept as a fallback for older messages.The implementation keeps the request flow explicit. packages/botkit/src/quote.ts defines the policy and request surface, while packages/botkit/src/quote-impl.ts owns the Accept/Reject behavior and stamp creation.
Bot.onQuoteRequeststill runs after automatic handling, because applications may want to observe accepted/rejected requests as well as moderate pending ones. TheQuoteRequest.statefield makes that distinction visible without asking applications to infer it from side effects.Accepted requests store
QuoteAuthorizationstamps in the repository layer. The memory, SQLite, and PostgreSQL repositories all index stamps by the remote quote object URI, because that is the stable handle a bot is likely to have when it later needs to revoke a quote. Duplicate requests preserve the first stamp so repeated delivery and concurrent delivery stay idempotent. Revocation also works when the caller only has the quote URI and the remote quote cannot be fetched anymore; the local stamp is removed first, then BotKit sends the best federation notification it can.Dynamic bot groups use the same target resolution path as other inbox activities, so quote requests for dynamically resolved actors are not limited to statically registered bots. PostgreSQL schema initialization now takes an advisory transaction lock around the schema DDL as well, which avoids catalog races when multiple test/runtime processes initialize the same schema at once.
The public API is documented in docs/concepts/bot.md, docs/concepts/events.md, and docs/concepts/message.md. The changelog entry in CHANGES.md records the new quote policy API, the quote request event, the revocation method, and the repository storage additions.
Closes #28.
Summary by CodeRabbit
onQuoteRequestdecision hook.quotePolicy, plus quote-authorization and unauthorization flows.quote) with quote request/policy types and policy normalization/serialization helpers, and wired new bot/event handlers.