Skip to content

fix(gui): protect parked prompt-layer drafts - #5368

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/parked-layer-drafts
Closed

luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/parked-layer-drafts

Conversation

@luvs01

@luvs01 luvs01 commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The prompt-layer editor's in-dialog navigation parked drafts in memory, but the close/save guard only considered the currently visible layer, so edits parked on other layers were silently discarded on close or save.
  • Track parked drafts per layer and whether any parked draft differs from its persisted sibling (parkedDirty), comparing against the saved layers via othersRef without widening effect deps.
  • Route close and save through requestClose/requestSave with a discardAction state, so the existing discard confirmation appears whenever a parked draft is dirty — including a Save initiated from another layer.
  • Add a regression test exercising both the close and save paths and verifying the server PUT behavior.

Verification

  • cd gui && bun test tests/codex-set-stack.test.tsx — 14 pass, 0 fail (includes the new regression test).
  • bun x tsc --noEmit (root) and cd gui && bun x tsc -b — both clean.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 20, 2026
@github-actions

github-actions Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • UI screenshot required.

What to do

  • Add a screenshot of the UI change to the PR description.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 58 / 80

이 PR은 커스텀 프롬프트 레이어 편집 창에서, 레이어를 넘나들며 잠깐 넣어 둔(parked) 수정이 닫기·저장 때 조용히 사라지던 구멍을 막습니다. 예전에는 지금 화면에 보이는 레이어만 dirty로 봤어요. 다른 레이어에 넣어 둔 글은 메모리 Map에만 있고, 닫거나 다른 레이어를 저장하면 확인 없이 버려졌습니다. 지금은 저장된 형제 레이어(others)와 비교해 parked 쪽에 다른 내용이 있으면 parkedDirty로 표시하고, 닫기·저장 모두 기존 버리기 확인(discardAction)을 거치게 합니다. 회귀 테스트 8b가 닫기·저장 양쪽에서 확인 UI가 뜨는지, 저장 확인 후에야 PUT이 나가는지 봅니다. base는 dev이고, 작성자 기준 관련 bun 테스트 14개와 tsc는 통과했다고 합니다. 아직 draft이고 본문 체크리스트는 0/4입니다. types/config 분할이나 미리보기 배포 이야기는 해당 없습니다. 같은 주제로 겹치는 열린 PR은 없습니다.

라인 gui/src/components/codex-set/CustomLayerDialog.tsx requestSave / discardAction === "save" — 저장을 눌렀을 때도 문구는 그대로 Discard your changes?이고, 빨간 버튼 라벨도 Discard입니다. 그런데 그 버튼을 누르면 parked 수정은 버리고 지금 보이는 레이어는 onSave로 저장합니다. 닫기 경로의 “버리기”와 같은 말이 저장 경로에서는 “다른 레이어 수정은 버리고 이 레이어만 저장”이 됩니다. 사용자가 Save를 눌렀는데 Discard를 확인해야 저장이 이어져서, 의미가 뒤집혀 보입니다.

라인 gui/src/components/codex-set/CustomLayerDialog.tsx setParkedDirty — dirty 여부는 editingId/layer가 바뀔 때만 다시 계산합니다. othersRefothers는 따라가지만, 부모가 같은 편집 중 others만 갱신해도 parkedDirty는 다시 안 돕니다. 흔한 경로는 아니지만 플래그가 한 박자 늦을 수 있습니다.

라인 gui/tests/codex-set-stack.test.tsx 8b — 닫기에서는 확인 UI가 뜨는지만 보고, Discard 후 실제로 닫히는지·PUT이 없는지는 안 봅니다. 저장 쪽 PUT 0→1은 잠깁니다. Keep editing 뒤 parked 본문이 그대로인지는 기존 테스트 8에 기대고, 저장 경고 경로와는 이어지지 않습니다.

메인테이너의 판단이 필요한 지점

저장 확인에서 parked 레이어는 항상 버리고 현재 레이어만 쓸지, “다시 편집” 외에 parked를 저장하는 길이 필요한지 정해 주세요. 지금 UX는 데이터 손실을 막되, 확인 문구는 닫기용 버리기 문장을 그대로 씁니다. 저장용 문구·버튼(예: 다른 레이어 수정을 버리고 저장)을 나눌지, 아니면 저장 전에는 확인만 하고 진행 라벨을 Save로 둘지 정책이 필요합니다.

너의 추천

방향과 base dev는 맞습니다. 머지 전에 (1) discardAction === "save"일 때 안내 문장과 확인 버튼 라벨을 저장 의미에 맞게 나누고, (2) 가능하면 테스트 8b에 닫기 Discard 후 에디터가 닫히고 PUT이 없는 단언을 보태세요. 문구/라벨이 정리되기 전에는 draft를 유지하고, 체크리스트·UI 스크린샷은 그 다음에 채우면 됩니다. 무효·중복으로 닫을 다른 열린 PR은 없습니다.

이 댓글은 grok-bot이 작성했습니다

In-dialog navigation parked drafts in memory, but the close/save guard only considered the currently visible layer, so edits parked on other layers were silently discarded. Track whether any parked draft differs from its persisted sibling and route close and save through the existing discard confirmation so unsaved custom prompt-layer text is never lost without a prompt.
@luvs01
luvs01 force-pushed the fix/parked-layer-drafts branch from 77558d7 to 23cf7a3 Compare September 21, 2026 13:14
@luvs01

luvs01 commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto current dev; gui tests pass (14/14). This change is behavior-only (parked draft protection in CustomLayerDialog) with no visual difference, so there is no meaningful screenshot to attach — requesting \gui-screenshot-waived\ from a maintainer if the gate requires it. cc @lidge-jun

@luvs01

luvs01 commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Consolidated into #5540 in native Stack #5505.

Source head: 23cf7a3ebc60bfe3dde267f46ee6bb406c5fb4cc. Replacement head: e352aee0343548a606a8d071b3504848c7503a41.

All 1 unique source contribution commit(s) match their carried commits by stable Git patch ID. The follow-up distinguishes Save confirmation from close/discard in all ten locales and tests parked-edit preservation and zero PUT on close. Prepared aggregate checks passed 200 runtime/configuration tests and 20 GUI/locale tests. Combined route/probe rerun passed 119 tests with one POSIX skip; combined GUI tests, typecheck, structure, privacy and ratchet checks passed. Full cross-platform CI, POSIX execution, docs build and independent review remain pending.

Closing this duplicate standalone review entry at the author's request after verifying migration. This is not a merge or release claim; remaining integration checks and reviews are tracked on the draft replacement. Original branches are retained.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants