Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 77 additions & 24 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
@@ -1,44 +1,97 @@
name: Claude Code Review
name: Claude PR Review

on:
pull_request:
types: [opened, synchronize, ready_for_review, reopened]
# Optional: Only run on specific file changes
# paths:
# - "src/**/*.ts"
# - "src/**/*.tsx"
# - "src/**/*.js"
# - "src/**/*.jsx"

# Only review the latest push: rapid pushes to the same PR would otherwise
# spawn overlapping runs that race on the tracking comment and burn
# Max-subscription quota.
concurrency:
group: claude-review-${{ github.event.pull_request.number }}
cancel-in-progress: true

jobs:
claude-review:
# Optional: Filter by PR author
# if: |
# github.event.pull_request.user.login == 'external-contributor' ||
# github.event.pull_request.user.login == 'new-developer' ||
# github.event.pull_request.author_association == 'FIRST_TIME_CONTRIBUTOR'

# Skip draft PRs, and skip fork PRs: forked pull_request runs get no
# repo secrets and a read-only GITHUB_TOKEN, so the job would fail
# loudly on auth instead of skipping cleanly.
if: >-
github.event.pull_request.draft == false &&
github.event.pull_request.head.repo.full_name == github.repository
runs-on: ubuntu-latest
timeout-minutes: 15
permissions:
contents: read
pull-requests: write
issues: write
id-token: write

actions: read
steps:
- name: Checkout repository
uses: actions/checkout@v4
uses: actions/checkout@v6
with:
fetch-depth: 1

- name: Run Claude Code Review
id: claude-review
- name: Claude PR Review
uses: anthropics/claude-code-action@v1
with:
# Authenticates against Mary's Claude Max subscription (OAuth token,
# not an API key). Secret set at repo level: CLAUDE_CODE_OAUTH_TOKEN.
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
plugin_marketplaces: 'https://github.com/anthropics/claude-code.git'
plugins: 'code-review@claude-code-plugins'
prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }}'
# See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md
# or https://code.claude.com/docs/en/cli-reference for available options

# Use the workflow's own token so we don't need to install the
# third-party Claude GitHub App on the org. Requires the job
# permissions block below (pull-requests: write).
github_token: ${{ secrets.GITHUB_TOKEN }}

# Live progress checklist comment on the PR while reviewing.
track_progress: true

prompt: |
REPO: ${{ github.repository }}
PR NUMBER: ${{ github.event.pull_request.number }}

You are reviewing the KeepSimpleOSS codebase: a Next.js **Pages
Router** app (React 19, TypeScript with strict off), styled with
SCSS Modules. See AGENTS.md for the full conventions.

Review this PR and focus on:

1. Correctness & React best practices
- Hooks rules, effect dependencies, stale closures
- Unnecessary re-renders, missing keys, prop drilling
- SSR/hydration safety: no `window`/`localStorage`/`document`
at module top level (guard in effects or use ssr:false)
2. TypeScript quality
- Avoid `any`, prefer precise types, exhaustive unions
3. Project conventions (AGENTS.md) — flag violations:
- App Router patterns (`'use client'`, `next/navigation`,
`src/app/`)
- Tailwind, styled-components, CSS-in-JS, or inline styles
- New state libraries (Redux, Zustand, Jotai, SWR, React Query)
- Global CSS imported anywhere except `_app.tsx`
- `<img src={svg}>` instead of importing SVGs as components
- Named exports from `index.ts` barrels, or empty barrels
- Import-order / path-alias violations
- Changes to UX Core bias data, slugs, or schema (these need
explicit approval — flag, don't wave through)
4. Accessibility & UX
- Semantic HTML, aria attributes, keyboard nav
5. Security
- XSS via dangerouslySetInnerHTML, unsanitized input,
leaked secrets/env, unsafe URL handling
6. Styling
- SCSS module hygiene; no hardcoded colors/spacing/breakpoints
that bypass the design tokens (keepsimple-style)

Leave inline comments for specific issues via the inline-comment
tool. Put your overall assessment and any praise in the tracking
comment summary. Be concise and actionable; skip nitpicks that a
linter would catch.

# Only the PR/commit-scoped inline-comment tool is granted. We
# deliberately do NOT grant raw `gh pr comment/view/diff`: those are
# unscoped, and since the review reads untrusted PR content (diff,
# description) a prompt-injection payload could steer them at other
# PRs/issues. PR context + diff are already injected via track_progress.
claude_args: |
--allowedTools "mcp__github_inline_comment__create_inline_comment"
4 changes: 3 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

MemPalace wing: `keepsimple` (protocol lives in `~/.claude/CLAUDE.md`).

Human-readable agent guidelines live in `AGENTS.md` next to this file; this file is the machine-facing version. See `AGENTS.md` for repo conventions, build/test commands, and contribution rules.
Human-readable agent guidelines live in `AGENTS.md` next to this file; this file is the machine-facing version. See `AGENTS.md` for repo conventions, build/test commands, and contribution rules — imported below so it loads automatically.

@AGENTS.md

## Code search — prefer CodeGraph over Grep

Expand Down
Binary file not shown.
16 changes: 14 additions & 2 deletions src/api/library/tag/getTagsList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,21 @@ export interface GetTagsListResponse {
data: ITag[];
}

export const getTagsList = async (): Promise<GetTagsListResponse> => {
// Tags are owner-scoped: each is stamped with `user` on create. The default
// GET /api/tags returns every account's tags, so always filter by the current
// user's id. Without an id there's nothing safe to return — refuse rather than
// fall back to the unscoped list, which would leak other accounts' tags.
export const getTagsList = async (
userId?: number | string,
): Promise<GetTagsListResponse> => {
if (userId == null || userId === '') {
return { data: [] };
}

try {
const { data } = await axiosInstance.get<GetTagsListResponse>('/api/tags');
const { data } = await axiosInstance.get<GetTagsListResponse>('/api/tags', {
params: { 'filters[user][id][$eq]': userId },
});

return data;
} catch (error) {
Expand Down
21 changes: 21 additions & 0 deletions src/components/Header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type { TRouter } from '@local-types/global';
import useGlobals from '@hooks/useGlobals';
import { useIsWidthLessThan } from '@hooks/useScreenSize';

import { getMyLibrary } from '@api/library/getMyLibrary';
import { userInfoUpdate } from '@api/settings';
import { getMyInfo } from '@api/strapi';

Expand Down Expand Up @@ -51,6 +52,24 @@ const Header: FC = () => {
const canCreateLibrary =
accountData?.featureNames?.includes('can-create-library') ?? false;

// "My Library" is only reachable once a library exists, or could be
// bootstrapped by a flag-holder. With neither, the user has no library page,
// so the dropdown item is disabled. Check via the owner-scoped lookup.
const [hasLibrary, setHasLibrary] = useState(false);
useEffect(() => {
if (!accountData?.id) {
setHasLibrary(false);
return;
}
let cancelled = false;
getMyLibrary(accountData.id).then(lib => {
if (!cancelled) setHasLibrary(lib !== null);
});
return () => {
cancelled = true;
};
}, [accountData?.id]);

useEffect(() => {
const storedToken = localStorage.getItem('accessToken');
setToken(storedToken);
Expand Down Expand Up @@ -167,6 +186,7 @@ const Header: FC = () => {
userImage={accountData?.picture}
handleOpenSettings={handleOpenSettings}
canCreateLibrary={canCreateLibrary}
hasLibrary={hasLibrary}
hideDropdown={isOpenedSidebar}
hideUsername
/>
Expand Down Expand Up @@ -247,6 +267,7 @@ const Header: FC = () => {
userImage={accountData?.picture}
handleOpenSettings={handleOpenSettings}
canCreateLibrary={canCreateLibrary}
hasLibrary={hasLibrary}
/>
)}
</div>
Expand Down
17 changes: 15 additions & 2 deletions src/components/UserProfile/UserProfile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type UserProfileProps = {
hideDropdown?: boolean;
hideUsername?: boolean;
canCreateLibrary?: boolean;
hasLibrary?: boolean;
setAccountData?: (updater: (prev: boolean) => boolean) => void;
setOpenLoginModal?: (openModal: boolean) => void;
handleOpenSettings?: () => void;
Expand Down Expand Up @@ -56,6 +57,7 @@ const UserProfile: FC<UserProfileProps> = ({
hideDropdown,
hideUsername,
canCreateLibrary,
hasLibrary,
setAccountData,
setOpenLoginModal,
handleOpenSettings,
Expand Down Expand Up @@ -84,10 +86,15 @@ const UserProfile: FC<UserProfileProps> = ({
handleOpenSettings?.();
}, [handleOpenSettings]);

// With neither an existing library nor create permission, the user has no
// library page to open, so the item is inert.
const myLibraryDisabled = !hasLibrary && !canCreateLibrary;

const handleMyLibrary = useCallback(() => {
if (myLibraryDisabled) return;
setIsDropdownOpen(false);
router.push(`/library/${username}`);
}, [router, username]);
}, [router, username, myLibraryDisabled]);

// A library has no standalone create step — it's bootstrapped on the owner's
// own page once they add content (gated server-side by the same feature
Expand Down Expand Up @@ -174,7 +181,13 @@ const UserProfile: FC<UserProfileProps> = ({
{isDropdownOpen && isAccessTokenExist && (
<div className={styles.dropdown} onClick={e => e.stopPropagation()}>
{isLibraryEnabled() && username && (
<div className={styles.menuItem} onClick={handleMyLibrary}>
<div
className={cn(styles.menuItem, {
[styles.disabled]: myLibraryDisabled,
})}
onClick={handleMyLibrary}
aria-disabled={myLibraryDisabled}
>
<LibraryIcon
width={20}
height={11}
Expand Down
2 changes: 2 additions & 0 deletions src/components/library/molecules/Button/Button.module.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
.button {
gap: 8px;
height: 44px;
box-sizing: border-box;
border: none;
cursor: pointer;
display: flex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@

.trigger {
width: 100%;
height: 44px;
box-sizing: border-box;
display: flex;
align-items: center;
justify-content: space-between;
gap: 8px;
background: var(--white);
border: 1px solid var(--brown-border);
border-radius: 4px;
padding: 12px 16px;
padding: 0 16px;
cursor: pointer;
color: var(--black);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

.trigger {
width: 100%;
height: 44px;
box-sizing: border-box;
padding: 0 0 0 12px;
display: flex;
align-items: center;
Expand Down Expand Up @@ -38,7 +40,6 @@
.text {
flex: 1;
text-align: left;
padding: 12px 0;
}

.iconWrapper {
Expand Down
3 changes: 2 additions & 1 deletion src/components/library/molecules/Input/Input.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@

.input {
width: 100%;
height: 35px;
height: 44px;
color: var(--black);
font-size: 16px;
padding: 0 16px;
background: var(--white);
border: 1px solid var(--gray-100);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@
top: 18px;
left: 46px;
right: 46px;
width: 79.5%;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

width: 79.5% over-constrains this absolutely-positioned element alongside left: 46px / right: 46px. Per CSS 10.3.7, when left, right, and width are all non-auto on an absolutely positioned box, right is recomputed from left + width and effectively ignored — so the symmetric "46px = circle + gap on each side" anchoring the comment above describes no longer holds. On any viewport where the parent width isn't the exact one this 79.5% was tuned against, the right end of the line will drift off the 46px anchor (and vice versa for left in RTL). Recommend dropping the width line (letting left/right alone determine the box) or removing right if width is the intended fix, and updating the comment either way.

Fix this →

height: 1px;
background: var(--gray-100);
overflow: hidden;
Expand Down
3 changes: 3 additions & 0 deletions src/components/library/molecules/Tag/Tag.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@

.text {
display: inline;
// Beats the global `.library p` serif rule (0,2,0 > 0,1,1) so tag labels
// use the sans face.
font-family: var(--font-source-sans);
}

.removeButton {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ export function AddObjectModal(props: AddObjectModalProps): JSX.Element {

useEffect(() => {
let cancelled = false;
getTagsList().then(res => {
getTagsList(accountData?.id).then(res => {
if (cancelled) return;
const opts: TagOption[] = res.data.map(t => ({
id: t.id,
Expand All @@ -293,7 +293,7 @@ export function AddObjectModal(props: AddObjectModalProps): JSX.Element {
return () => {
cancelled = true;
};
}, []);
}, [accountData?.id]);

// Preset the object's existing tags exactly once, from the object's OWN
// populated tag data — not by filtering the fetched options. An unpublished
Expand Down
6 changes: 5 additions & 1 deletion src/components/library/organisms/LibraryCard/LibraryCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ export function LibraryCard(props: LibraryCardProps): JSX.Element {
const router = useRouter();

const handleViewLibrary = () => {
router.push(`/library/${username ?? id}`);
// Route by numeric id, not username: the route resolver short-circuits a
// numeric param to a findOne-by-id, sidestepping the username→id filter
// lookup that the public API currently 500s on. Falls back to username only
// if an id is somehow absent.
router.push(`/library/${id ?? username}`);
};

return (
Expand Down
Loading
Loading