Skip to content

fix: PR #45 코드리뷰 3건 반영 (greet 제거·시뮬레이션 버전·credential 폴백) - #46

Merged
wook10 merged 2 commits into
mainfrom
fix/45-review-greet-flush-credentials
Sep 17, 2026
Merged

wook10 merged 2 commits into
mainfrom
fix/45-review-greet-flush-credentials

Conversation

@wook10

@wook10 wook10 commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

배경

#45 (exijn 님의 Poppy-Server 계약 정렬 PR) 에 대한 inryeok-bot 리뷰가 아직 반영되지
않은 채로 머지돼 있어, main 에서 새 브랜치를 파서 3건을 마저 반영합니다.

반영한 리뷰

  1. HIGH · BEHAVIOR — INITIAL_PROGRAM(기본 블록 프로그램)에 있던 greet('인사하기')
    블록을 뺐습니다. 실제 Poppy-Server 블록 계약엔 greet 에 대응하는 타입이 없어
    (toServerBlockProgram 이 명시적으로 거부) 새 세션의 첫 자동 저장부터 실패하는
    문제였습니다. 이미 저장된 구버전 로컬 초안 등 다른 경로로 greet 가 남아 있을 경우를
    대비해, 스택·자유 블록 어디에 있든 greet 가 있으면 막는 'unsupported-block' 검증도
    추가했습니다(서버가 지원하게 되면 SERVER_UNSUPPORTED_KINDS 에서 빼면 됩니다).
  2. HIGH · DATA_INTEGRITY (ExperienceView.tsx:145) — runSimulation() 이 블록
    버전이 아직 0(한 번도 저장 안 됨)일 때만 강제로 저장하던 걸, 마지막으로 저장한
    프로그램과 지금 화면이 다를 때도
    즉시 저장하도록 고쳤습니다. 600ms 자동 저장
    debounce 가 아직 안 끝난 상태에서 시뮬레이션을 돌리면, 통과 기록이 방금 검증한
    프로그램이 아니라 그 이전 버전을 가리킬 수 있었습니다.
  3. MEDIUM · ERROR_HANDLING (sessionCredentials.ts:41) — writeSessionCredentials
    가 localStorage 쓰기에 실패해도(사파리 프라이빗 모드, 저장 공간 초과 등) 조용히
    credential 을 잃어버리던 걸, 모듈 스코프 메모리 캐시로 대체하도록 고쳤습니다. 이번
    탭 안에서는 localStorage 유무와 상관없이 계속 인증 헤더를 만들 수 있습니다.

테스트

  • blockProgram.test.ts — unsupported-block 검증(스택·자유 블록 양쪽) 신규 3건 +
    INITIAL_PROGRAM 구성 변경(greet 제거)에 맞춰 기존 fixture 조정.
  • blockDnd.test.tsx — 같은 이유로 인사하기 를 참조하던 드래그·삭제 테스트를 종료
    기준으로 다시 작성(그린 조각1 "블록 삭제" 기능은 그대로 유지).
  • ExperienceView.test.tsx — "시뮬레이션 직전 아직 자동 저장 안 된 편집이 있으면, 먼저
    저장한 최신 버전으로 통과 기록을 남긴다" 신규 통합 테스트 1건.
  • sessionCredentials.test.ts (신규 파일) — localStorage 정상 동작 + 읽기/쓰기 실패 시
    메모리 폴백 6건 + 테스트 격리용 __resetSessionCredentialsForTests 테스트.

npm run typecheck && npm run lint && npm run test -- --run && npm run build && npm run format:check
전부 통과 확인(105 passed, 1 skipped).

🤖 Generated with Claude Code

…실패 대비

- HIGH·BEHAVIOR: INITIAL_PROGRAM 에 있던 'greet' 블록을 뺀다 — 실제 Poppy-Server 블록
  계약엔 대응 타입이 없어(toServerBlockProgram 이 거부) 그 상태로는 첫 자동 저장부터
  실패한다. 이미 로컬에 남아있는 초안(구버전) 대비, greet 가 스택·자유 블록 어디든
  있으면 막는 'unsupported-block' 검증도 추가했다.
- HIGH·DATA_INTEGRITY: runSimulation() 이 버전 0일 때만 강제 저장하던 걸, 마지막으로
  저장한 프로그램과 지금 화면이 다를 때도(600ms 자동 저장 debounce 가 안 끝났을 수
  있음) 즉시 저장하도록 고쳤다 — 안 그러면 통과 기록이 방금 시뮬레이션한 프로그램이
  아닌 이전 버전을 가리킬 수 있다.
- MEDIUM·ERROR_HANDLING: sessionCredentials 에 모듈 스코프 메모리 캐시를 추가해,
  localStorage 쓰기/읽기가 실패해도(프라이빗 모드, 용량 초과 등) 이번 탭 안에서는
  credential 을 계속 쓸 수 있게 했다.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@inryeok-bot inryeok-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

리뷰 결과

변경된 7개 파일을 검토했으며, 수정이 필요한 문제 2개를 발견했습니다.

심각도 개수
Critical 0
High 0
Medium 2
Low 0

주요 내용

  • MEDIUM · DATA_INTEGRITY — 강제 저장 결과가 아닌 이전 `currentVersion`으로 통과 기록을 남깁니다
  • MEDIUM · SECURITY — 정상적으로 삭제된 세션도 메모리 캐시에서 되살아납니다

검토한 head: fc41a2e79da4

// (600ms 자동 저장 debounce 가 아직 안 끝났을 수 있다) 지금 바로 저장해 그 버전을 쓴다.
// 안 그러면 시뮬레이션은 최신 프로그램으로 통과했는데 통과 기록은 그 이전(구버전)에
// 남아, 실제 실행이 방금 검증한 것과 다른 이전 버전으로 나갈 수 있다(코드리뷰 발견).
if (session.data && (currentVersion === 0 || program !== savedProgramRef.current)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MEDIUM · DATA_INTEGRITY

강제 저장 결과가 아닌 이전 `currentVersion`으로 통과 기록을 남깁니다

편집 직후 시뮬레이션하면 이 분기에서 새 revision을 저장하지만, await 이후에도 현재 이벤트 핸들러가 캡처한 currentVersion은 이전 렌더의 값입니다. 따라서 기존 버전이 1인 상태에서 새 버전 2를 저장해도 simulationPass에는 1이 전달되고, 최초 저장처럼 값이 0이었다면 통과 기록 자체가 생기지 않습니다.

saveBlockRevision.mutateAsync()의 반환값에서 blockVersion을 받아 지역 변수로 사용하고, 저장하지 않은 경우에만 기존 currentVersion을 사용해야 합니다.

Comment thread src/shared/api/sessionCredentials.ts Outdated
} catch {
return null;
}
return memoryCredentials;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MEDIUM · SECURITY

정상적으로 삭제된 세션도 메모리 캐시에서 되살아납니다

getItem()이 오류 없이 null을 반환한 경우까지 memoryCredentials로 fallback합니다. 그래서 같은 origin의 다른 탭에서 clearSessionCredentials()를 호출해 공유 localStorage를 지워도, 이미 열려 있던 탭은 이전 credential을 계속 읽고 X-Session-Token을 전송합니다.

메모리 fallback은 storage 접근이 예외로 실패했거나 이 탭의 저장이 실패한 경우에만 사용하고, 성공적인 null 읽기는 세션 부재로 처리해야 합니다. 다중 탭 삭제를 지원하려면 storage 이벤트에서도 메모리 값을 비워야 합니다.

- MEDIUM·DATA_INTEGRITY: runSimulation() 이 "저장이 필요한지" 조건만 보고 그 결과에
  따라 flushSave() 호출 여부를 결정하던 걸, 조건은 "새로 큐에 넣을지"만 결정하고
  flushSave() 자체는 세션이 있으면 항상 기다리도록 고쳤다. 디바운스가 방금 스스로
  저장을 시작했지만 아직 안 끝난 순간에 시뮬레이션하면, 예전 코드는 "이미 같다"고
  보고 그냥 이전 버전으로 통과 기록을 남겼다 — flushSave() 는 진행 중인 저장이 있으면
  그걸 기다렸다가 정확한 최종 버전을 돌려주므로, 항상 기다리는 쪽이 맞다.
- MEDIUM·SECURITY: sessionCredentials 의 메모리 캐시가 "localStorage 를 정상적으로
  읽었는데 비어있는" 경우까지 대신 써서, 다른 탭에서 로그아웃 등으로 공유 localStorage
  를 지워도 이미 열려 있던 탭은 이전 credential 을 계속 보냈다. 이제 메모리 캐시는
  "이 탭에서 직전 쓰기가 실패했을 때"만 빈 읽기를 대신하고, 그 외의 정상적인 빈 읽기는
  그대로 세션 없음으로 처리한다. 다른 탭의 삭제를 더 빨리 반영하도록 storage 이벤트로도
  메모리 캐시를 비운다.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@wook10
wook10 merged commit c8ea3d3 into main Sep 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant