Skip to content

fix(ui): save note drafts without stale input races - #1050

Merged
benvinegar merged 2 commits into
mainfrom
fix/stale-draft-input-race
Sep 8, 2026
Merged

fix(ui): save note drafts without stale input races#1050
benvinegar merged 2 commits into
mainfrom
fix/stale-draft-input-race

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

  • scope textarea updates and saves to the draft instance that emitted them
  • flush the focused editor value before Ctrl-S or mouse saves consume the draft
  • keep draft identities unique when notes are reopened within one millisecond
  • cover stale callbacks and fast-save behavior at unit and PTY levels

Stack

Validation

  • bun run format:check
  • bun run lint
  • bun run typecheck
  • bun run deps:check
  • bun run changeset:status
  • bun run test (2,100 passed)
  • bun run test:integration (163 passed, 1 platform skip)
  • bun run test:tty-smoke (10 passed)
  • rapid-save and delayed-input PTY regressions repeated 10 times each
  • real TTY smoke on an actual commit diff
  • bun run build:npm && bun run check:pack

@vercel

vercel Bot commented Sep 8, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
hunk-web Ignored Ignored Sep 8, 2026 4:34am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens note composition against delayed textarea events and fast-save races by associating updates with draft identities and synchronously flushing the focused editor before saving.

  • Adds monotonically suffixed identities for newly created, edited, and reply drafts.
  • Rejects updates and saves emitted by stale draft callbacks.
  • Flushes live textarea content for keyboard and mouse saves before consuming semantic draft state.
  • Adds unit and PTY coverage for delayed updates, replacement drafts, and rapid saves.

Confidence Score: 5/5

The PR appears safe to merge; draft identity checks and synchronous store updates address the stale-input race without an established regression.

The semantic store applies editor-body updates synchronously before saving, stale callbacks are rejected using unique draft identities, and the changed behavior is covered at composer, review-controller, and PTY levels.

Important Files Changed

Filename Overview
packages/hunk/src/ui/hooks/useTerminalReview.ts Adds unique draft-instance identities and synchronously rejects body updates targeting a stale draft.
packages/hunk/src/ui/hooks/useUserNoteComposer.ts Scopes editor callbacks to captured draft identities and flushes current editor content before saving.
packages/hunk/src/ui/hooks/useAppKeyboardShortcuts.ts Supplies the focused editor’s live text to the note-save shortcut.
packages/hunk/src/ui/components/panes/AgentInlineNote.tsx Supplies the draft textarea’s live text when the mouse save action fires.
packages/hunk/src/ui/hooks/useTerminalReview.test.tsx Verifies delayed updates cannot mutate saved or replacement drafts.
packages/hunk/src/ui/hooks/useUserNoteComposer.test.tsx Covers save-time flushing and rejected stale callbacks.
test/pty/notes.test.ts Exercises the delayed textarea-event race through the terminal input path.

Sequence Diagram

sequenceDiagram
  participant E as Draft editor
  participant C as Note composer
  participant R as Review store
  participant P as Event publisher
  E->>C: Save(current editor body)
  C->>R: Update body(expected draft ID)
  alt Draft ID matches
    R-->>C: Accepted
    C->>R: Consume and save draft
    R-->>C: Saved note
    C->>P: Publish committed note
  else Callback belongs to stale draft
    R-->>C: Rejected
    C-->>E: Abort save
  end
Loading

Reviews (1): Last reviewed commit: "test(ui): reject stale draft saves" | Re-trigger Greptile

@benvinegar
benvinegar merged commit 4f95ca5 into main Sep 8, 2026
22 checks 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