Skip to content

[refactor] Header 컴포넌트 UI/비즈니스 로직 분리#1394

Open
seongwon030 wants to merge 11 commits intodevelop-fefrom
refactor/#1393-header-logic-separation-MOA-789
Open

[refactor] Header 컴포넌트 UI/비즈니스 로직 분리#1394
seongwon030 wants to merge 11 commits intodevelop-fefrom
refactor/#1393-header-logic-separation-MOA-789

Conversation

@seongwon030
Copy link
Copy Markdown
Member

@seongwon030 seongwon030 commented Apr 7, 2026

#️⃣연관된 이슈

ex) #1393

📝작업 내용

  • useHeaderNavigationhandleMenuClose 추가 — 모바일 메뉴 닫기 Mixpanel 트래킹을 Header.tsx에서 훅으로 이관
  • useHeaderVisibility 훅 신규 생성 — showOn/hideOn 기반 렌더 조건 판단 로직 분리
  • DeviceTypesrc/types/device.ts로 공통화
  • Header.tsx에서 useDevice, isInAppWebView, shouldRender, getCurrentDeviceTypes 제거

중점적으로 리뷰받고 싶은 부분(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?

논의하고 싶은 부분(선택)

논의하고 싶은 부분이 있다면 작성해주세요.

🫡 참고사항

Summary by CodeRabbit

  • Documentation

    • Header 컴포넌트 아키텍처와 동작(표시 조건, 내비게이션, 스크롤 감지 등)을 설명하는 문서를 추가했습니다.
  • New Features

    • 기기 및 웹뷰 상태에 따라 헤더 표시/숨김을 중앙에서 제어하는 가시성 로직을 도입했습니다.
    • 메뉴 닫기 동작에 대한 전환 처리와 관련 이벤트 추적 핸들러를 추가했습니다.
  • Tests

    • 다양한 기기 및 웹뷰 시나리오에 대한 헤더 가시성 단위 테스트를 추가했습니다.
  • Chores

    • 개발 도구용 훅/스크립트와 프로젝트 메타데이터 및 무시 규칙을 추가했습니다.

모바일 메뉴 닫기 Mixpanel 트래킹을 Header에서 useHeaderNavigation으로 이관
- DeviceType을 src/types/device.ts로 공통화
- useHeaderVisibility 훅으로 showOn/hideOn 판단 로직 분리
- Header.tsx에서 useDevice, isInAppWebView, shouldRender 제거
showOn/hideOn/webview/우선순위 등 10개 케이스 커버
@seongwon030 seongwon030 self-assigned this Apr 7, 2026
@seongwon030 seongwon030 added 🔨 Refactor 코드 리팩토링 💻 FE Frontend labels Apr 7, 2026
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
moadong Ready Ready Preview, Comment Apr 8, 2026 2:53am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 7, 2026

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Validation error: Invalid regex pattern for base branch. Received: "**" at "reviews.auto_review.base_branches[0]"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

Header 렌더링 가시성 로직이 컴포넌트에서 분리되어 useHeaderVisibility 훅으로 이동했습니다. 메뉴 닫기 추적은 useHeaderNavigationhandleMenuClose로 이동했고, 관련 문서와 단위 테스트가 추가되었습니다. DeviceType 타입이 새로 도입되었습니다.

Changes

Cohort / File(s) Summary
Header 가시성 훅 + 타입
frontend/src/hooks/Header/useHeaderVisibility.ts, frontend/src/types/device.ts
기기 유형(mobile,tablet,laptop,desktop,webview)을 기반으로 헤더 표시 여부를 반환하는 useHeaderVisibility(showOn?, hideOn?) 훅과 DeviceType 타입을 추가했습니다.
Header 컴포넌트
frontend/src/components/common/Header/Header.tsx
컴포넌트 내 기기 감지 및 showOn/hideOn 판정을 제거하고 useHeaderVisibility의 반환값(isVisible)으로 렌더링을 제어하도록 변경. 메뉴 닫기 시 기존 Mixpanel 직접 호출을 제거하고 useHeaderNavigationhandleMenuClose를 사용하도록 수정했습니다.
네비게이션 훅 확장
frontend/src/hooks/Header/useHeaderNavigation.ts
handleMenuClose 콜백을 추가하여 메뉴 닫기 이벤트를 트래킹(USER_EVENT.MOBILE_MENU_DELETE_BUTTON_CLICKED)하도록 확장하고 반환 객체에 포함시켰습니다.
테스트 & 문서
frontend/src/hooks/Header/useHeaderVisibility.test.ts, frontend/docs/features/components/header.md
useHeaderVisibility에 대한 유닛 테스트 스위트와 Header 아키텍처/사용 문서를 추가했습니다. (테스트 파일에 한 곳의 잘못된 괄호 존재)
기타 리포지토리 메타/훅
.gitignore, frontend/.claude/hooks/*, frontend/.claude/settings.json, frontend/.omc/*
.omc 메타데이터, Claude 관련 훅/설정 스크립트 및 .gitignore.omc/ 추가 등 개발 도구/메타파일이 새로 추가되었습니다.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Header as Header.tsx
    participant Visibility as useHeaderVisibility
    participant DeviceSvc as useDevice / isInAppWebView
    participant NavHook as useHeaderNavigation
    participant Tracker as trackEvent

    User->>Header: 페이지 로드 / 상호작용
    Header->>Visibility: useHeaderVisibility(showOn, hideOn)
    Visibility->>DeviceSvc: useDevice(), isInAppWebView()
    DeviceSvc-->>Visibility: current device types
    Visibility-->>Header: isVisible (true/false)
    alt isVisible == false
        Header-->>User: null (렌더링 없음)
    else isVisible == true
        Header-->>User: 헤더 렌더링
    end

    User->>Header: 메뉴 닫기 클릭
    Header->>NavHook: handleMenuClose()
    NavHook->>Tracker: trackEvent(USER_EVENT.MOBILE_MENU_DELETE_BUTTON_CLICKED)
    Tracker-->>NavHook: ack
    NavHook-->>Header: 완료
    Header-->>User: 메뉴 닫힘 반영
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • lepitaaar
  • suhyun113
  • oesnuj
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목은 주요 변경사항인 Header 컴포넌트의 UI와 비즈니스 로직 분리를 명확하게 요약하고 있으며, 실제 변경 내용과 일치합니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/#1393-header-logic-separation-MOA-789

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

🎨 UI 변경사항을 확인해주세요

변경된 스토리를 Chromatic에서 확인해주세요.

구분 링크
🔍 변경사항 리뷰 https://www.chromatic.com/build?appId=67904e61c16daa99a63b44a7&number=155
📖 Storybook https://67904e61c16daa99a63b44a7-gffailpjhs.chromatic.com/

2개 스토리 변경 · 전체 56개 스토리 · 22개 컴포넌트

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
frontend/src/hooks/Header/useHeaderVisibility.test.ts (1)

147-160: hideOn=[] + showOn 조합 회귀 테스트를 추가해 주세요.

현재 우선순위 테스트는 잘 되어 있지만, 빈 hideOn 배열 케이스가 없어 Line 17-18 경계조건 회귀를 잡기 어렵습니다.

테스트 추가 예시
 describe('hideOn이 showOn보다 우선순위가 높다', () => {
   it('hideOn과 showOn이 동시에 있을 때 hideOn이 우선 적용된다', () => {
@@
     expect(result.current).toBe(false);
   });
+
+  it('hideOn이 빈 배열이면 showOn 조건을 적용한다', () => {
+    // Given
+    setupDevice({ isDesktop: false, isMobile: true });
+
+    // When
+    const { result } = renderHook(() =>
+      useHeaderVisibility(['desktop'], []),
+    );
+
+    // Then
+    expect(result.current).toBe(false);
+  });
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/hooks/Header/useHeaderVisibility.test.ts` around lines 147 -
160, Add a regression test in useHeaderVisibility.test that covers the edge case
where hideOn is an empty array while showOn has values: call
useHeaderVisibility([], ['desktop']) (using setupDevice({ isDesktop: true }))
and assert the expected visibility (likely true) to ensure the Line 17-18
boundary condition is covered; place the test near the existing priority test
("hideOn이 showOn보다 우선순위가 높다") and reference the hook useHeaderVisibility and
renderHook/result.current in the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/components/common/Header/Header.tsx`:
- Around line 52-55: 현재 메뉴 닫기 이벤트 로깅은 closeMenu()에서만 수행되어 네비 링크 클릭 흐름(라인
80-83)에서는 handleMenuClose()가 호출되지만, 메뉴 버튼의 실제 닫기 동작(toggleMenu())에서는 호출되지 않아 이벤트
수집이 누락됩니다; toggleMenu()가 메뉴를 닫을 때도 동일한 종료 후처리(예: handleMenuClose 또는 closeMenu)를
호출하도록 변경하거나 closeMenu의 로직을 분리해 공용 함수(handleMenuClose)를 실제 닫기 경로(현재 closeMenu와
toggleMenu 둘 다)에 호출하게 수정하여 이벤트 수집이 일관되게 작동하도록 하세요 (참조: closeMenu, toggleMenu,
handleMenuClose, setIsMenuOpen).

In `@frontend/src/hooks/Header/useHeaderVisibility.ts`:
- Around line 17-18: The hook useHeaderVisibility incorrectly treats an empty
hideOn array as "present" and returns early; change the existence checks to
require non-empty arrays (e.g., use hideOn && hideOn.length > 0 and showOn &&
showOn.length > 0) so that hideOn only short-circuits when it actually contains
items, preserving evaluation of showOn and correct use of currentTypes with the
existing hideOn/showOn logic.

---

Nitpick comments:
In `@frontend/src/hooks/Header/useHeaderVisibility.test.ts`:
- Around line 147-160: Add a regression test in useHeaderVisibility.test that
covers the edge case where hideOn is an empty array while showOn has values:
call useHeaderVisibility([], ['desktop']) (using setupDevice({ isDesktop: true
})) and assert the expected visibility (likely true) to ensure the Line 17-18
boundary condition is covered; place the test near the existing priority test
("hideOn이 showOn보다 우선순위가 높다") and reference the hook useHeaderVisibility and
renderHook/result.current in the assertion.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ec948af2-6ef0-4b36-b964-320d1b4fdf44

📥 Commits

Reviewing files that changed from the base of the PR and between a2dca15 and 9af707b.

📒 Files selected for processing (6)
  • frontend/docs/features/components/header.md
  • frontend/src/components/common/Header/Header.tsx
  • frontend/src/hooks/Header/useHeaderNavigation.ts
  • frontend/src/hooks/Header/useHeaderVisibility.test.ts
  • frontend/src/hooks/Header/useHeaderVisibility.ts
  • frontend/src/types/device.ts

네비 링크 클릭이 아닌 실제 메뉴 버튼 닫기 시에만 이벤트가 발동되도록 수정.
setIsMenuOpen updater 내 prev 기반으로 열림→닫힘 전환 시에만 handleMenuClose 호출.
hideOn 존재 여부 대신 length로 체크해 hideOn=[]일 때 showOn이 정상 평가되도록 수정.
경계 조건 테스트 3개 추가.
- Add 5 OMC session JSON files recording session end reasons
- Add HUD state and stdin cache snapshots
- Add frontend/.claude hooks (stop.sh, post-compact.sh) for auto-commit and context re-injection
- Add frontend/.claude/settings.json with PostToolUse, Stop, Notification, PostCompact hooks
- Add frontend/.omc project memory and agent replay logs
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/.claude/hooks/post-compact.sh`:
- Line 3: The CLAUDE_MD variable in frontend/.claude/hooks/post-compact.sh is
hardcoded to a user-specific absolute path; change it to resolve a
repository-relative path or accept an environment override so the hook works for
other developers/CI. Specifically, update the CLAUDE_MD assignment (variable
name CLAUDE_MD in post-compact.sh) to prefer an environment-provided value
(e.g., honour $CLAUDE_MD if set) and otherwise compute the location relative to
the script directory (derive the script's directory and join with the relative
path to CLAUDE.md) instead of the current "/Users/..." literal.
- Line 10: The CONTEXT extraction uses an unstable termination pattern '^##
[^아]' which can misidentify the next section; update the awk invocation that
sets CONTEXT (the line using CLAUDE_MD) to use a stateful approach: match the
start header '^## 아키텍처 개요', set a flag, print lines while the flag is set, and
clear the flag when any next header matching '^## ' is seen so the section
reliably ends at the next '##' header; keep the existing head -60 truncation if
desired and ensure the variable names CONTEXT and CLAUDE_MD are used unchanged.

In `@frontend/.claude/hooks/stop.sh`:
- Around line 13-15: The hook currently stages all changes with "git add -A" in
stop.sh which can include untracked or accidental files; change the staging
command to a safer alternative such as "git add -u" to only update tracked files
or implement an explicit allowlist of paths before staging, and update the
stop.sh logic that runs "git add -A" to use the new command or allowlist check
so only intended files are staged.
- Around line 27-34: The current COMMIT_MSG assignment sends the entire staged
DIFF to an external tool (claude) via the COMMIT_MSG=$(echo "$DIFF" | claude
...) pipeline, which can leak secrets; change stop.sh so sending the full DIFF
is opt-in and/or masked: gate the claude call behind an explicit environment
flag (e.g., require ALLOW_SEND_DIFF or similar) before using COMMIT_MSG and,
when enabled, preprocess DIFF to remove or redact sensitive patterns (e.g.,
lines containing "password", "secret", "api_key", "BEGIN PRIVATE KEY", private
key blocks, or files like .env) before piping to claude; update the logic that
sets COMMIT_MSG to use the gated+filtered DIFF instead of raw $DIFF (referencing
the COMMIT_MSG assignment and DIFF variable).
- Around line 10-11: REPO_ROOT assignment can fail under set -u if
CLAUDE_PROJECT_DIR is unset; change the fallback to use a safe default expansion
(use CLAUDE_PROJECT_DIR with a default empty value) and add an explicit
validation after assignment: if REPO_ROOT is empty, log an error and exit
non‑zero; keep the existing cd "$REPO_ROOT" || exit 0 but only after the
emptiness check. Update the assignment that sets REPO_ROOT and add the
post‑assignment check referencing REPO_ROOT and CLAUDE_PROJECT_DIR so the script
won't abort unexpectedly under set -u.

In `@frontend/.claude/settings.json`:
- Line 9: The command currently feeds tool_input.file_path directly into
prettier allowing files outside the repo; update the shell snippet that builds
"command" to resolve the input with realpath (e.g. realpath "$f") and compare it
against the repository root realpath (resolve the hardcoded project root used in
the cd, e.g. realpath "/Users/seokyoung-won/Desktop/moadong/frontend"); only run
npx prettier --write "$f" if the resolved path is a descendant of the repo root
(prefix check), otherwise skip and return a non-error status or log a warning;
ensure the check is applied where tool_input.file_path is read so the rest of
the pipeline behavior is unchanged.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9894d8ba-ba10-4b95-a108-6c3d8309daa4

📥 Commits

Reviewing files that changed from the base of the PR and between 7ebfabc and 1beb95b.

📒 Files selected for processing (7)
  • .gitignore
  • frontend/.claude/hooks/post-compact.sh
  • frontend/.claude/hooks/stop.sh
  • frontend/.claude/settings.json
  • frontend/.omc/project-memory.json
  • frontend/.omc/state/agent-replay-6b6a7221-468f-4720-b656-788fd4544f85.jsonl
  • frontend/.omc/state/agent-replay-a9f7de0a-c7be-4097-a51a-bf05146e6fc3.jsonl
✅ Files skipped from review due to trivial changes (4)
  • frontend/.omc/state/agent-replay-a9f7de0a-c7be-4097-a51a-bf05146e6fc3.jsonl
  • .gitignore
  • frontend/.omc/state/agent-replay-6b6a7221-468f-4720-b656-788fd4544f85.jsonl
  • frontend/.omc/project-memory.json

@@ -0,0 +1,18 @@
#!/bin/bash
# PostCompact hook: CLAUDE.md 핵심 컨텍스트를 압축 후 재주입
CLAUDE_MD="/Users/seokyoung-won/Desktop/moadong/frontend/CLAUDE.md"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

절대경로 하드코딩으로 훅이 다른 환경에서 동작하지 않습니다

Line 3의 사용자 로컬 절대경로는 다른 개발자/CI에서 즉시 깨집니다. 스크립트 위치 기준 상대경로(또는 환경변수 우선)로 바꿔주세요.

제안 패치
-CLAUDE_MD="/Users/seokyoung-won/Desktop/moadong/frontend/CLAUDE.md"
+SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
+PROJECT_ROOT="$(cd -- "$SCRIPT_DIR/../.." && pwd)"
+CLAUDE_MD="${CLAUDE_MD:-$PROJECT_ROOT/CLAUDE.md}"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/.claude/hooks/post-compact.sh` at line 3, The CLAUDE_MD variable in
frontend/.claude/hooks/post-compact.sh is hardcoded to a user-specific absolute
path; change it to resolve a repository-relative path or accept an environment
override so the hook works for other developers/CI. Specifically, update the
CLAUDE_MD assignment (variable name CLAUDE_MD in post-compact.sh) to prefer an
environment-provided value (e.g., honour $CLAUDE_MD if set) and otherwise
compute the location relative to the script directory (derive the script's
directory and join with the relative path to CLAUDE.md) instead of the current
"/Users/..." literal.

fi

# 아키텍처 개요 섹션만 추출 (너무 많은 토큰 방지)
CONTEXT=$(awk '/^## 아키텍처 개요/,/^## [^아]/' "$CLAUDE_MD" | head -60)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

아키텍처 섹션 종료 조건이 불안정합니다

Line 10의 종료 패턴(^## [^아])은 “다음 ## 헤더”를 정확히 의미하지 않아 섹션 경계가 잘못 잡힐 수 있습니다. 시작 헤더 진입 후 다음 ## 에서 종료하도록 상태 기반으로 바꾸는 게 안전합니다.

제안 패치
-CONTEXT=$(awk '/^## 아키텍처 개요/,/^## [^아]/' "$CLAUDE_MD" | head -60)
+CONTEXT=$(
+  awk '
+    /^## 아키텍처 개요$/ { in_section=1; print; next }
+    /^## / && in_section { exit }
+    in_section { print }
+  ' "$CLAUDE_MD" | head -60
+)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CONTEXT=$(awk '/^## 아키텍처 개요/,/^## [^아]/' "$CLAUDE_MD" | head -60)
CONTEXT=$(
awk '
/^## 아키텍처 개요$/ { in_section=1; print; next }
/^## / && in_section { exit }
in_section { print }
' "$CLAUDE_MD" | head -60
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/.claude/hooks/post-compact.sh` at line 10, The CONTEXT extraction
uses an unstable termination pattern '^## [^아]' which can misidentify the next
section; update the awk invocation that sets CONTEXT (the line using CLAUDE_MD)
to use a stateful approach: match the start header '^## 아키텍처 개요', set a flag,
print lines while the flag is set, and clear the flag when any next header
matching '^## ' is seen so the section reliably ends at the next '##' header;
keep the existing head -60 truncation if desired and ensure the variable names
CONTEXT and CLAUDE_MD are used unchanged.

Comment on lines +10 to +11
REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) || REPO_ROOT="$CLAUDE_PROJECT_DIR"
cd "$REPO_ROOT" || exit 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

set -u 환경에서 fallback 변수가 비어 있으면 스크립트가 종료될 수 있습니다

Line 10은 CLAUDE_PROJECT_DIR가 unset일 때 비정상 종료될 수 있습니다. 안전한 기본값 처리 후 REPO_ROOT 유효성 검사를 추가해 주세요.

제안 패치
-REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) || REPO_ROOT="$CLAUDE_PROJECT_DIR"
-cd "$REPO_ROOT" || exit 0
+REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || true)"
+REPO_ROOT="${REPO_ROOT:-${CLAUDE_PROJECT_DIR:-}}"
+[ -n "$REPO_ROOT" ] || exit 0
+cd "$REPO_ROOT" || exit 0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) || REPO_ROOT="$CLAUDE_PROJECT_DIR"
cd "$REPO_ROOT" || exit 0
REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || true)"
REPO_ROOT="${REPO_ROOT:-${CLAUDE_PROJECT_DIR:-}}"
[ -n "$REPO_ROOT" ] || exit 0
cd "$REPO_ROOT" || exit 0
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/.claude/hooks/stop.sh` around lines 10 - 11, REPO_ROOT assignment
can fail under set -u if CLAUDE_PROJECT_DIR is unset; change the fallback to use
a safe default expansion (use CLAUDE_PROJECT_DIR with a default empty value) and
add an explicit validation after assignment: if REPO_ROOT is empty, log an error
and exit non‑zero; keep the existing cd "$REPO_ROOT" || exit 0 but only after
the emptiness check. Update the assignment that sets REPO_ROOT and add the
post‑assignment check referencing REPO_ROOT and CLAUDE_PROJECT_DIR so the script
won't abort unexpectedly under set -u.

Comment on lines +13 to +15
# Stage all changes
git add -A 2>/dev/null || true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

세션 종료 시 git add -A는 과도하게 넓은 범위를 커밋합니다

Line 13-15는 추적되지 않은 파일/실수로 생성된 파일까지 포함할 수 있어 사고 위험이 큽니다. 최소 git add -u로 제한하거나 명시적 allowlist가 필요합니다.

제안 패치
-# Stage all changes
-git add -A 2>/dev/null || true
+# Stage tracked changes only (safer default)
+git add -u 2>/dev/null || true
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Stage all changes
git add -A 2>/dev/null || true
# Stage tracked changes only (safer default)
git add -u 2>/dev/null || true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/.claude/hooks/stop.sh` around lines 13 - 15, The hook currently
stages all changes with "git add -A" in stop.sh which can include untracked or
accidental files; change the staging command to a safer alternative such as "git
add -u" to only update tracked files or implement an explicit allowlist of paths
before staging, and update the stop.sh logic that runs "git add -A" to use the
new command or allowlist check so only intended files are staged.

Comment on lines +27 to +34
COMMIT_MSG=$(echo "$DIFF" | claude -p \
"You are a commit message generator. Based on the following git diff, write a single commit message.
Rules:
- First line MUST start with 'WIP(scope): short summary' (max 72 chars)
- Always use 'WIP' as the type prefix, never feat/fix/refactor/etc.
- If needed, add a blank line then bullet points for details
- Be concise and specific
- Output ONLY the commit message, nothing else" 2>/dev/null) || true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

staged diff 전체를 외부 도구로 전송하는 동작은 민감정보 유출 위험이 있습니다

Line 27-34는 코드/비밀값이 포함된 diff를 그대로 전송할 수 있습니다. 기본 비활성화(옵트인) 또는 민감패턴 마스킹이 필요합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/.claude/hooks/stop.sh` around lines 27 - 34, The current COMMIT_MSG
assignment sends the entire staged DIFF to an external tool (claude) via the
COMMIT_MSG=$(echo "$DIFF" | claude ...) pipeline, which can leak secrets; change
stop.sh so sending the full DIFF is opt-in and/or masked: gate the claude call
behind an explicit environment flag (e.g., require ALLOW_SEND_DIFF or similar)
before using COMMIT_MSG and, when enabled, preprocess DIFF to remove or redact
sensitive patterns (e.g., lines containing "password", "secret", "api_key",
"BEGIN PRIVATE KEY", private key blocks, or files like .env) before piping to
claude; update the logic that sets COMMIT_MSG to use the gated+filtered DIFF
instead of raw $DIFF (referencing the COMMIT_MSG assignment and DIFF variable).

"hooks": [
{
"type": "command",
"command": "jq -r '.tool_input.file_path // empty' | { read -r f; [ -n \"$f\" ] && cd /Users/seokyoung-won/Desktop/moadong/frontend && npx prettier --write \"$f\" --ignore-unknown 2>/dev/null; } || true",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

tool_input.file_path 경로 검증이 없어 repo 바깥 파일 대상 실행 가능성이 있습니다

Line 9는 입력 경로를 바로 prettier에 전달합니다. realpath 기준으로 프로젝트 루트 하위인지 확인 후 실행하도록 제한하는 게 안전합니다.

제안 패치(개념)
-"command": "jq -r '.tool_input.file_path // empty' | { read -r f; [ -n \"$f\" ] && cd /Users/seokyoung-won/Desktop/moadong/frontend && npx prettier --write \"$f\" --ignore-unknown 2>/dev/null; } || true",
+"command": "jq -r '.tool_input.file_path // empty' | { read -r f; root=\"${CLAUDE_PROJECT_DIR:-$(pwd)}\"; [ -n \"$f\" ] || exit 0; abs=\"$(realpath \"$f\" 2>/dev/null || true)\"; case \"$abs\" in \"$root\"/*) cd \"$root\" && npx prettier --write \"$abs\" --ignore-unknown 2>/dev/null ;; esac; } || true",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"command": "jq -r '.tool_input.file_path // empty' | { read -r f; [ -n \"$f\" ] && cd /Users/seokyoung-won/Desktop/moadong/frontend && npx prettier --write \"$f\" --ignore-unknown 2>/dev/null; } || true",
"command": "jq -r '.tool_input.file_path // empty' | { read -r f; root=\"${CLAUDE_PROJECT_DIR:-$(pwd)}\"; [ -n \"$f\" ] || exit 0; abs=\"$(realpath \"$f\" 2>/dev/null || true)\"; case \"$abs\" in \"$root\"/*) cd \"$root\" && npx prettier --write \"$abs\" --ignore-unknown 2>/dev/null ;; esac; } || true",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/.claude/settings.json` at line 9, The command currently feeds
tool_input.file_path directly into prettier allowing files outside the repo;
update the shell snippet that builds "command" to resolve the input with
realpath (e.g. realpath "$f") and compare it against the repository root
realpath (resolve the hardcoded project root used in the cd, e.g. realpath
"/Users/seokyoung-won/Desktop/moadong/frontend"); only run npx prettier --write
"$f" if the resolved path is a descendant of the repo root (prefix check),
otherwise skip and return a non-error status or log a warning; ensure the check
is applied where tool_input.file_path is read so the rest of the pipeline
behavior is unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

💻 FE Frontend 🔨 Refactor 코드 리팩토링

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant