feat(api-service,human): cap keyless human interactions and push the claim link - #3
Open
evanmarshall wants to merge 1 commit into
Open
feat(api-service,human): cap keyless human interactions and push the claim link#3evanmarshall wants to merge 1 commit into
evanmarshall wants to merge 1 commit into
Conversation
…claim link A keyless `@novu/human` environment could create interactions forever: the 24h keyless expiry is only enforced on the Inbox session path, and the existing keyless reply cap lives in the agents inbound-turn handler, which `POST /v1/human/interactions` never goes through. After `KEYLESS_HUMAN_INTERACTION_CAP` interactions (default 5, every kind counts) the API no longer delivers the prompt. It sends a "Sign up & keep this setup" card with the connect claim link on the channel the prompt would have used (once per environment) and returns a 429 whose body carries `code: KEYLESS_HUMAN_CAP_REACHED`, `claimUrl`, and `cap`, mirroring the `novu connect` keyless CTA. Claiming now also moves HumanInteraction rows, and a keyless credential for an already-claimed environment gets a 403 pointing at `human setup --secret-key` instead of a misleading "run setup". The CLI detects the structured 429, prints the claim link and the recovery command, and exits 1 per the existing contract. README and the agent skill doc describe the limit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Hey there and thank you for opening this pull request! 👋 We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted. Your PR title is: Requirements:
Expected format: Details: PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name |
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
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.
Mirror of novuhq#12544 for Ito QA evaluation.
Upstream PR: novuhq#12544
Upstream author: @scopsy
Upstream head commit:
8e1e7e00845abe1aee013b195e2c676aaa6b12c1What changed? Why was the change needed?
A keyless
@novu/humanenvironment (npx @novu/human setupwithout an account) could create interactions forever. The 24h keyless expiry is only enforced on the Inbox session path, and the existingKEYLESS_DEMO_REPLY_CAPgate lives in the agents inbound-turn handler, whichPOST /v1/human/interactionsnever goes through.This adds a server-side cap to the human interactions path and pushes the connect claim link, mirroring the
novu connectkeyless CTA:API
CreateInteractioncounts every interaction row in a keyless environment (includingtelland the setup smoke test). PastKEYLESS_HUMAN_INTERACTION_CAP(default 5) it does not deliver the prompt. It sends a "Sign up & keep this setup" card with the claim link on the channel the prompt would have used, once per environment so a retrying agent does not spam the human, and returns a 429 with a structured body:code: KEYLESS_HUMAN_CAP_REACHED,claimUrl,cap, plus a message that includes the link.ClaimKeylessConnectnow also movesHumanInteractionrows, sohuman wait <id>/human listkeep working after the claim.human setup --secret-key <key>/NOVU_SECRET_KEY, instead of the misleading "Relay agent not found. Runhuman setup" (which would mint a fresh keyless env).ConnectClaimTokenService.isEnvironmentClaimed,HumanDeliveryService.deliverContent,buildKeylessHumanSignupCard, andresolveKeylessHumanInteractionCap(read at call time so it is testable and tunable).CLI (
packages/human)code, falling back to the message wording) and prints the claim link plus the recovery command. Exit code stays1per the existing contract.SKILL.mddescribe the limit and tell agents to stop retrying and surface the link.Out of scope, as discussed: a
human logindevice-auth flow so the CLI can switch to the claimed environment without pasting a key; a "N free messages left" hint; enforcing the 24h keyless expiry inKeylessStrategy.Screenshots
N/A (API + CLI).
Expand for optional sections
Related enterprise PR
None.
Special notes for your reviewer
human-interactions.e2e.ts(cap + card once + no rows persisted, non-keyless untouched, claim moves rows and stale credential gets 403) and 6 new unit cases increate-interaction.usecase.spec.ts. Full human e2e file (24) and the human CLI vitest suite (47) pass.create-interaction.usecase.spec.ts("keeps the row, stamps successful deliveries…" and "moves the primary subscriber…") fail onnextwithout this change: they assert a top-levelsubscriberIdon the stamp call that the lifecycle stopped writing in refactor(api-service,dal,shared): collapse HumanInteraction onto deliveries fixes NV-8703 novuhq/novu#12478. Not touched here.POST /v1/human/interactions. In-threadctx.ask-style interactions from framework agents already sit behind the existing keyless reply cap.🤖 Generated with Claude Code
Greptile Summary
This PR caps keyless human interactions, sends a claim CTA when the allowance is exhausted, migrates human-interaction rows during claim, and teaches the CLI how to present recovery guidance.
Confidence Score: 2/5
The PR should not merge until the keyless cap and CTA deduplication are made concurrency-safe and claimed environments can be recognized after transient claim-token state disappears.
Concurrent interaction requests can exceed the configured allowance and duplicate the supposedly one-time CTA, while cache expiry causes stale claimed credentials to revert to the misleading setup path.
Files Needing Attention: apps/api/src/app/human/usecases/create-interaction/create-interaction.usecase.ts; apps/api/src/app/connect/services/connect-claim-token.service.ts
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[POST human interaction] --> B{Keyless organization?} B -->|No| G[Resolve and deliver interaction] B -->|Yes| C{Claim cache says claimed?} C -->|Yes| D[Return 403 re-auth guidance] C -->|No| E[Count environment interactions] E -->|Below cap| G E -->|At cap| F[Issue claim link and send CTA] F --> H[Return structured 429] I[Connect claim] --> J[Move agents, channels, and human interactions] J --> K[Consume claim token]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(api-service,human): cap keyless hum..." | Re-trigger Greptile