Conversation
🦋 Changeset detectedLatest commit: 004b05d The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Preview URLsUse these stable preview aliases for testing this PR:
These point to the same Vercel preview deployment as the bot comment, but they stay stable and easier to find. Raw Vercel deployment URLs
|
|
TL;DR — Reverts PR #3012 (which itself reverted #1474), restoring the original removal of manual Key changes
Summary ｜ 108 files ｜ 2 commits ｜ base:
The change is mechanical and applies uniformly: every
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean revert of #3012, effectively re-applying the useMemo/useCallback removal from #1474. The biome config on main already bans these imports, so this brings the code into compliance. All transformations are correct — plain expressions replace useMemo, function declarations replace useCallback, and biome-ignore comments are added where functions legitimately appear in dependency arrays. The channel-defaults-section.tsx refactoring is the most structural change and correctly isolates the 'use no memo' directive to the minimal ChannelDefaultsSectionInner component.
Claude Opus ｜ 𝕏
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This PR cleanly reverts PR #3012 to restore the project's React Compiler-based memoization policy. The changes are mechanical and aligned with the established codebase conventions documented in CLAUDE.md.
🔴❗ Critical (0) ❗🔴
None.
🟠⚠️ Major (0) 🟠⚠️
None.
🟡 Minor (0) 🟡
None.
💭 Consider (1) 💭
Inline Comments:
- 💭 Consider:
.changeset/beige-bottles-tease.md:4Remove ignored package@inkeep/agents-docsfrom changeset
✅ APPROVE
Summary: This is a clean revert that correctly restores the React Compiler memoization policy. The biome.jsonc change re-enables the lint rule banning useCallback/useMemo imports, and all ~108 component/hook files are properly converted back to regular functions. The React Compiler (enabled for this project) will handle memoization automatically. One minor suggestion: the changeset includes @inkeep/agents-docs which is in the ignore list — this is harmless but slightly noisy.
Discarded (5)
| Location | Issue | Reason Discarded |
|---|---|---|
sidebar.tsx:88-101 |
useEffect function dependency pattern | Valid pattern with React Compiler — reviewer self-assessed as "no change needed" |
use-chat-activities-polling.ts:192-200 |
Hook returns functions recreated on render | Valid pattern with React Compiler — compiler handles memoization |
timeline-wrapper.tsx:227-242 |
Activities computed on every render | Valid pattern with React Compiler — compiler auto-memoizes based on deps |
slack-provider.tsx:62-80 |
Context value created inline | Valid pattern with React Compiler — compiler memoizes context values |
sidebar.tsx:103-116 |
Context value without useMemo | Valid pattern with React Compiler — same as above |
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-frontend |
5 | 0 | 0 | 0 | 0 | 0 | 5 |
pr-review-devops |
2 | 0 | 1 | 0 | 1 | 0 | 0 |
| Total | 7 | 0 | 1 | 0 | 1 | 0 | 5 |
Note: Frontend findings were all self-assessed by the reviewer as "correct patterns with React Compiler — no change needed", so they were discarded as non-issues.
| --- | ||
| "@inkeep/agents-manage-ui": patch | ||
| "@inkeep/agents-cli": patch | ||
| "@inkeep/agents-docs": patch |
There was a problem hiding this comment.
💭 Consider: Remove ignored package from changeset
Issue: @inkeep/agents-docs is listed in the changeset but is in .changeset/config.json's ignore list. Changesets will skip it during versioning anyway.
Why: While harmless, including ignored packages adds noise and may confuse contributors about what actually gets released.
Fix:
| "@inkeep/agents-docs": patch | |
| "@inkeep/agents-manage-ui": patch | |
| "@inkeep/agents-cli": patch |
Refs:
- .changeset/config.json:10-14 — ignore list
|
🔎💬 Inkeep AI search and chat service is syncing content for source 'Inkeep Agent Framework Docs' |
Ito Test Report ✅18 test cases ran. 1 additional finding, 17 passed. The unified run covered 18 scenarios with 17 passes, confirming stable behavior across auth and invitation routing, device authorization (including malformed input handling), Slack link happy path, open-redirect defenses, traces/date/mobile/query-string resilience (with only controlled upstream telemetry-related errors), GitHub installation actions under rapid stress, members role and row-action integrity, submit idempotency, and cross-tab/back-forward session stability. The only failure was a medium-severity backend issue where a previously consumed Slack link token is still accepted on replay (POST /work-apps/slack/users/link/verify-token returns 200 due to JWT/schema validation without one-time-use tracking and an idempotent existing-link success path), weakening one-time-link security though identified as a pre-existing defect rather than introduced by this PR. ✅ Passed (17)
ℹ️ Additional Findings (1)
🟠 Replayed Slack link token is accepted after first use
Relevant code:
export async function verifySlackLinkToken(token: string): Promise<VerifySlackLinkTokenResult> {
const result = await verifyJwt(token, { issuer: ISSUER, audience: AUDIENCE });
if (!result.valid || !result.payload) {
return {
valid: false,
error: result.error,
};
}
const parseResult = SlackLinkTokenPayloadSchema.safeParse(result.payload);
if (!parseResult.success) {
return {
valid: false,
error: `Invalid token schema: ${parseResult.error.issues.map((e) => e.message).join(', ')}`,
};
}
return {
valid: true,
payload: parseResult.data,
};
}
const existingLink = await findWorkAppSlackUserMapping(runDbClient)(
tenantId,
slackUserId,
teamId,
'work-apps-slack'
);
if (existingLink && existingLink.inkeepUserId === inkeepUserId) {
return c.json({
success: true,
linkId: existingLink.id,
slackUsername: existingLink.slackUsername || undefined,
slackTeamId: teamId,
tenantId,
});
}Commit: Tell us how we did: Give Ito Feedback |
1 participant
No description provided.