Fix Slack agent billing and evlog context#447
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
The latest updates on your projects. Learn more about Unkey Deploy
|
Greptile SummaryThis PR fixes Slack/API-key agent billing by routing all org-scoped API keys through the organization owner, attaches the shared AI and RPC logging machinery to the Slack evlog context via
Confidence Score: 4/5Safe to merge with minor caveats — the core evlog context wiring is sound and the billing fix is well-tested, but the tsconfig change quietly removes an index-access safety net across the whole AI package. The async-context plumbing in evlog-slack is correct: both the AI and RPC logger providers catch exceptions from getActiveSlackLog so there are no uncaught throws outside a Slack context. The billing routing change is backed by new tests. The main concern is packages/ai/tsconfig.json dropping noUncheckedIndexedAccess to make the freshly-added check-types script pass — this suppresses a category of potential type errors across the entire package going forward rather than fixing the underlying issues. packages/ai/tsconfig.json (noUncheckedIndexedAccess disabled) and packages/ai/src/ai/agents/execution.ts (billing behavior now applies to all org-scoped API keys, not only Slack). Important Files Changed
Sequence DiagramsequenceDiagram
participant Slack
participant run-handler
participant evlog-slack
participant agent-client
participant execution
participant Autumn
Slack->>run-handler: handleAgentRun(run)
run-handler->>evlog-slack: createSlackEventLog(fields)
run-handler->>evlog-slack: withSlackLogContext(eventLog, fn)
activate evlog-slack
Note over evlog-slack: AsyncLocalStorage.run(eventLog, fn)
run-handler->>agent-client: agent.stream(run, options)
agent-client->>evlog-slack: setActiveSlackLog({agent_chat_id, org_id, api_key_id})
Note over evlog-slack: Enriches active logger via AsyncLocalStorage
agent-client->>execution: resolveAgentBillingCustomerId(principal)
alt apiKey has organizationId
execution->>execution: getOrganizationOwnerId(apiKeyOrganizationId)
Note over execution: Bills org owner for ALL org-scoped API keys
else no apiKey.organizationId
execution->>execution: getBillingCustomerId(userId, organizationId)
end
execution-->>agent-client: billingCustomerId
agent-client->>Autumn: check(customerId, agent_credits)
Autumn-->>agent-client: {allowed, balance}
agent-client->>evlog-slack: mergeWideEvent({credits context})
Note over run-handler,evlog-slack: AI/RPC loggers use getActiveSlackLog() provider
run-handler->>evlog-slack: eventLog.emit() [in finally]
deactivate evlog-slack
Reviews (1): Last reviewed commit: "fix(rpc): keep log context export lightw..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "isolatedModules": true, | ||
| "noEmit": true, | ||
| "noUncheckedIndexedAccess": true, | ||
| "noUncheckedIndexedAccess": false, |
There was a problem hiding this comment.
[featureId, value] destructuring in trackAgentUsageAndBill, path[0] in rpc-log-context.ts). Disabling it to make check-types pass trades away a whole class of type-safety guarantees rather than fixing the underlying typing. Consider fixing the narrowing errors instead and re-enabling this.
| "noUncheckedIndexedAccess": false, | |
| "noUncheckedIndexedAccess": true, |
| if (apiKeyOrganizationId) { | ||
| const customerId = await getOrganizationOwnerId(apiKeyOrganizationId); | ||
| mergeAgentBillingFields({ | ||
| apiKeyId: principal.apiKey?.id, | ||
| apiKeyUserId: principal.apiKey?.userId, | ||
| billingCustomerId: customerId, | ||
| organizationId: apiKeyOrganizationId, | ||
| resolution: customerId | ||
| ? "api_key_org_owner" | ||
| : "api_key_org_owner_missing", | ||
| }); | ||
| return customerId; | ||
| } |
There was a problem hiding this comment.
When apiKey.organizationId is present the function now unconditionally routes billing to the organization owner, even for API keys that also carry a userId. This matches the Slack fix described in the PR, but resolveAgentBillingCustomerId is shared by the MCP/dashboard paths too. Any non-Slack API key that previously billed against a specific user's Autumn customer record will silently shift to billing the org owner after this change. The new test explicitly asserts this behavior for the case where userId is populated, which confirms it is intentional — worth a callout in case other consumers rely on user-level billing.
1 participant
Summary
Verification